Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: capture errors that shouldnt cause the whole plan to be discarded during network resolution #444

Open
wants to merge 6 commits into
base: transition-to-runkit
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,21 @@ def minitest_set_options(test_task, name)
test_task.options = "#{TESTOPTS} #{minitest_args} -- --simplecov-name=#{name}"
end

def core(early_deploy: false)
def core(early_deploy: false, capture_errors_during_network_resolution: false)
s = ":no-early-deploy"
if early_deploy
if capture_errors_during_network_resolution
s = ":capture-errors"
files_setup = ["test/features/capture_errors.rb"]
elsif early_deploy
s = ":early-deploy"
early_deploy_setup = ["test/features/early_deploy.rb"]
files_setup = ["test/features/early_deploy.rb"]
end

Rake::TestTask.new("test:core#{s}") do |t|
t.libs << "."
t.libs << "lib"
minitest_set_options(t, "core")
test_files = FileList["test/**/test_*.rb", *early_deploy_setup]
test_files = FileList["test/**/test_*.rb", *files_setup]
test_files = test_files
.exclude("test/ros/**/*.rb")
.exclude("test/gui/**/*.rb")
Expand Down Expand Up @@ -78,9 +81,11 @@ Rake::TestTask.new("test:gui") do |t|
end

core early_deploy: true
core capture_errors_during_network_resolution: true
Comment on lines 83 to +84
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should run with both activated as well ... no ?

core
desc "Run core library tests, excluding GUI and live tests"
task "test:core" => ["test:core:no-early-deploy", "test:core:early-deploy"]
task "test:core" => ["test:core:no-early-deploy", "test:core:early-deploy",
"test:core:capture-errors"]

desc "Run all tests"
task "test" => ["test:gui", "test:core", "test:live", "test:telemetry"]
Expand Down
2 changes: 1 addition & 1 deletion lib/syskit/cli/doc/gen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def self.compute_system_network(model, main_plan = Roby::Plan.new, **options)
main_plan.add(original_task = model.as_plan)
engine = Syskit::NetworkGeneration::Engine.new(main_plan)
planning_task = original_task.planning_task
mapping = engine.compute_system_network([planning_task], **options)
mapping, = engine.compute_system_network([planning_task], **options)

if engine.work_plan.respond_to?(:commit_transaction)
engine.work_plan.commit_transaction
Expand Down
254 changes: 125 additions & 129 deletions lib/syskit/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -353,22 +353,20 @@ def pretty_print(pp)
# Exception raised when we could not find devices to allocate for tasks
# that are device drivers
class DeviceAllocationFailed < SpecError
# The set of tasks that failed allocation
attr_reader :failed_tasks
# The task that failed allocation
attr_reader :failed_task
# A task to parents mapping for tasks involved in this error, at the
# time of the exception creation
attr_reader :task_parents
# Existing candidates for this device
attr_reader :candidates

def initialize(plan, tasks)
@failed_tasks = tasks.dup
def initialize(plan, task)
@failed_task = task
@candidates = {}
@task_parents = {}

tasks.each do |abstract_task|
resolve_device_task(plan, abstract_task)
end
resolve_device_task(plan, task)
end

def resolve_device_task(plan, abstract_task)
Expand All @@ -383,7 +381,7 @@ def resolve_device_task(plan, abstract_task)
all_tasks |= candidates[srv]
end
end
self.candidates[abstract_task] = candidates
@candidates = candidates

all_tasks.each do |t|
next if task_parents.key?(t)
Expand All @@ -399,35 +397,33 @@ def resolve_device_task(plan, abstract_task)
end

def pretty_print(pp)
pp.text "cannot find a device to tie to #{failed_tasks.size} task(s)"
pp.text "cannot find a device to tie to a task"

failed_tasks.each do |task|
parents = task_parents[task]
candidates = self.candidates[task]
parents = task_parents[failed_task]
candidates = self.candidates

pp.breakable
pp.text "for #{task.to_s.gsub('Syskit::', '')}"
pp.nest(2) do
unless parents.empty?
pp.breakable
pp.seplist(parents) do |parent|
role, parent = parent
pp.text "child #{role.to_a.first} of #{parent.to_s.gsub('Syskit::', '')}"
end
pp.breakable
pp.text "for #{failed_task.to_s.gsub('Syskit::', '')}"
pp.nest(2) do
unless parents.empty?
pp.breakable
pp.seplist(parents) do |parent|
role, parent = parent
pp.text "child #{role.to_a.first} of #{parent.to_s.gsub('Syskit::', '')}"
end
end

pp.breakable
pp.seplist(candidates) do |cand|
srv, tasks = *cand
if tasks.empty?
pp.text "no candidates for #{srv.short_name}"
else
pp.text "candidates for #{srv.short_name}"
pp.nest(2) do
pp.breakable
pp.seplist(tasks) do |cand_t|
pp.text cand_t.to_s
end
pp.breakable
pp.seplist(candidates) do |cand|
srv, tasks = *cand
if tasks.empty?
pp.text "no candidates for #{srv.short_name}"
else
pp.text "candidates for #{srv.short_name}"
pp.nest(2) do
pp.breakable
pp.seplist(tasks) do |cand_t|
pp.text cand_t.to_s
end
end
end
Expand Down Expand Up @@ -467,147 +463,147 @@ def pretty_print(pp)
class ConflictingDeploymentAllocation < SpecError
include Syskit::NetworkGenerationsExceptionHelpers

attr_reader :deployment_to_tasks
attr_reader :orocos_name

def initialize(deployment_to_tasks, toplevel_tasks_to_requirements = {})
@deployment_to_tasks = deployment_to_tasks
def initialize(orocos_name, tasks, toplevel_tasks_to_requirements = {})
@orocos_name = orocos_name
@tasks = tasks
@toplevel_tasks_to_requirements = toplevel_tasks_to_requirements
@deployment_to_execution_agent = \
deployment_to_tasks.transform_values do |tasks|
tasks.first.execution_agent
end
@agent = tasks.first.execution_agent
end

def pretty_print(pp)
deployment_to_tasks.each do |orocos_name, tasks|
agent = @deployment_to_execution_agent[orocos_name]
deployment_m = agent.deployed_orogen_model_by_name(orocos_name)
pp.text(
"deployed task '#{orocos_name}' from deployment " \
"'#{deployment_m.name}' defined in " \
"'#{deployment_m.project.name}' on '#{agent.process_server_name}' " \
"is assigned to #{tasks.size} 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:"
deployment_m = @agent.deployed_orogen_model_by_name(orocos_name)
pp.text(
"deployed task '#{orocos_name}' from deployment " \
"'#{deployment_m.name}' defined in " \
"'#{deployment_m.project.name}' on '#{@agent.process_server_name}' " \
"is assigned to #{@tasks.size} 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:"
)
@tasks.each do |t|
defs = find_all_related_syskit_actions(
t, @toplevel_tasks_to_requirements
)
tasks.each do |t|
defs = find_all_related_syskit_actions(
t, @toplevel_tasks_to_requirements
)
print_dependent_definitions(pp, t, defs)
end
print_failed_merge_chain(pp, tasks[0], tasks[1])
print_dependent_definitions(pp, t, defs)
end
print_failed_merge_chain(pp, @tasks[0], @tasks[1])
end
end

# Exception raised at the end of #resolve if some tasks do not have a
# Exception raised at the end of #resolve if a task does not have a
# deployed equivalent
class MissingDeployments < SpecError
# The tasks that are not deployed, as a hash from the actual task to
class MissingDeployment < SpecError
# The tasks that is not deployed, as a hash from the actual task to
# a set of [role_set, parent_task] pairs
#
# This is computed in #initialize as the dependency structure will
# probably change afterwards
attr_reader :tasks

# The task that is not deployed
attr_reader :task

# Parent tasks of the not deployed task
attr_reader :parents
# All the available deployment candidates
attr_reader :candidates
# The deployment hints of the not deployed task
attr_reader :deployment_hints

# Initializes this exception by providing a mapping from tasks that
# have no deployments to the deployment candidates
# have no deployments to the deployment candidates, and the specific task the
# error refers to.
#
# @param [Hash{TaskContext=>[Array<Model<Deployment>>]}] tasks_with_candidates
def initialize(tasks_with_candidates)
@tasks = {}
tasks_with_candidates.each do |task_to_deploy, candidates|
parents = task_to_deploy.dependency_context
candidates = candidates.map do |deployed_task, existing_tasks|
existing_tasks = existing_tasks.map do |task|
[task, task.dependency_context]
end
[deployed_task, existing_tasks]
def initialize(task, candidates)
@task = task
@parents = task.dependency_context
@candidates = candidates.map do |deployed_task, existing_tasks|
existing_tasks = existing_tasks.map do |task|
[task, task.dependency_context]
end

@tasks[task_to_deploy] = [
parents, candidates, task_to_deploy.deployment_hints
]
[deployed_task, existing_tasks]
end
@deployment_hints = task.deployment_hints
end

def pretty_print(pp)
pp.text "cannot deploy the following tasks"
tasks.each do |task, (parents, possible_deployments)|
pp.text "cannot deploy the following task"
pp.breakable
pp.text "#{task} (#{task.orogen_model.name})"
pp.nest(2) do
pp.breakable
pp.text "#{task} (#{task.orogen_model.name})"
pp.nest(2) do
pp.breakable
pp.seplist(parents) do |parent_task|
role, parent_task = parent_task
pp.text "child #{role} of #{parent_task}"
end
pp.seplist(parents) do |parent_task|
role, parent_task = parent_task
pp.text "child #{role} of #{parent_task}"
end
end

tasks.each do |task, (_parents, possible_deployments, deployment_hints)|
has_free_deployment = possible_deployments.any? { |_, existing| existing.empty? }
pp.breakable
if has_free_deployment
pp.text "#{task}: multiple possible deployments, choose one with #prefer_deployed_tasks(deployed_task_name)"
unless deployment_hints.empty?
deployment_hints.each do |hint|
pp.text " current hints: #{deployment_hints.map(&:to_s).join(', ')}"
end
has_free_deployment = candidates.any? { |_, existing| existing.empty? }
pp.breakable
if has_free_deployment
pp.text "#{task}: multiple possible deployments, choose one with " \
"#prefer_deployed_tasks(deployed_task_name)"
unless deployment_hints.empty?
deployment_hints.each do |hint|
pp.text " current hints: " \
"#{deployment_hints.map(&:to_s).join(', ')}"
end
elsif possible_deployments.empty?
pp.text "#{task}: no deployments available"
else
pp.text "#{task}: some deployments exist, but they are already used in this network"
end
elsif candidates.empty?
pp.text "#{task}: no deployments available"
else
pp.text "#{task}: some deployments exist, but they are already used " \
"in this network"
end

pp.nest(2) do
possible_deployments.each do |deployed_task, existing|
pp.breakable
process_server_name = deployed_task.configured_deployment
.process_server_name
orogen_model = deployed_task.configured_deployment
.orogen_model
pp.text(
"task #{deployed_task.mapped_task_name} from deployment " \
"#{orogen_model.name} defined in " \
"#{orogen_model.project.name} on #{process_server_name}"
)
pp.nest(2) do
existing.each do |task, parents|
pp.breakable
msg = parents.map do |parent_task|
role, parent_task = *parent_task
"child #{role} of #{parent_task}"
end
pp.text "already used by #{task}: #{msg.join(', ')}"
pp.nest(2) do
candidates.each do |deployed_task, existing|
pp.breakable
process_server_name = deployed_task.configured_deployment
.process_server_name
orogen_model = deployed_task.configured_deployment
.orogen_model
pp.text(
"task #{deployed_task.mapped_task_name} from deployment " \
"#{orogen_model.name} defined in " \
"#{orogen_model.project.name} on #{process_server_name}"
)
pp.nest(2) do
existing.each do |task, parents|
pp.breakable
msg = parents.map do |parent_task|
role, parent_task = *parent_task
"child #{role} of #{parent_task}"
end
pp.text "already used by #{task}: #{msg.join(', ')}"
end
end
end
end
end
end

# Exception raised at the end of #resolve if some tasks are referring to non-existing
# configurations
# Exception raised at the end of #resolve if a task is referring to non-existing
# configuration
class MissingConfigurationSection < SpecError
# Association of tasks and the sections that are missing
attr_reader :missing_sections_by_task
# The sections that are missing
attr_reader :missing_sections
# The task with a missing configuration section
attr_reader :task

def initialize(missing_sections_by_task)
@missing_sections_by_task = missing_sections_by_task
def initialize(task, missing_sections)
@task = task
@missing_sections = missing_sections
end

def pretty_print(pp)
pp.text "the following configuration sections are used but do not exist"
@missing_sections_by_task.each do |task, sections|
pp.breakable
pp.text "'#{sections.join('\', \'')}', in use by:"
pp.breakable
pp.text " #{task} (#{task.orogen_model.name})"
end
pp.breakable
pp.text "'#{missing_sections.join('\', \'')}', in use by:"
pp.breakable
pp.text " #{task} (#{task.orogen_model.name})"
end
end

Expand Down
5 changes: 3 additions & 2 deletions lib/syskit/gui/component_network_base_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,9 @@ def compute_system_network(model, main_plan = nil)
main_plan.add(original_task = model.as_plan)
base_task = original_task.as_service
engine = Syskit::NetworkGeneration::Engine.new(main_plan)
engine.compute_system_network([base_task.task.planning_task])
base_task.task
_, resolution_errors =
engine.compute_system_network([base_task.task.planning_task])
[base_task.task, resolution_errors]
ensure
if engine && engine.work_plan.respond_to?(:commit_transaction)
engine.work_plan.commit_transaction
Expand Down
Loading