Skip to content

Commit

Permalink
feat: capture errors during network generation
Browse files Browse the repository at this point in the history 
Instead of raising we will capture them, which allows the generation
process to finish for all the tasks that dont contain any errors. For
those that do, these errors are propagated after the network generation
process ends.
  • Loading branch information
@jhonasiv
jhonasiv committed Feb 25, 2025
1 parent f7051df commit cafa22a
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 75 deletions.
38 changes: 30 additions & 8 deletions ruby/lib/transformer/syskit/extensions/system_network_generator.rb
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# frozen_string_literal: true

require "syskit/network_generation"

module Transformer
# Module used to add some functionality to Syskit::NetworkGeneration::Engine
module SystemNetworkGeneratorExtension
# During network validation, checks that all required frames have been
# configured
def validate_generated_network
def validate_generated_network(error_handler: @error_handler)
super

return unless Syskit.conf.transformer_enabled?
Expand All @@ -19,6 +23,10 @@ def transformer_validate_all_frames_assigned
tr.each_needed_transformation do |transform|
transformer_validate_frame_assigned(task, transform.from)
transformer_validate_frame_assigned(task, transform.to)
rescue BaseException => e
@error_handler.register_resolution_failures_from_exception(
[task], e, e.message
)
end
end
end
Expand All @@ -42,14 +50,17 @@ def transformer_validate_no_missing_transform
raise InvalidChain.new(
placeholder_task.transformer,
task, task_from, from, task_to, to, e
),
"cannot find a transformation chain to produce "\
"#{from} => #{to} for #{task} (task-local frames: "\
"#{task_from} => #{task_to}): #{e.message}", e.backtrace
), "cannot find a transformation chain to produce "\
"#{from} => #{to} for #{task} (task-local frames: "\
"#{task_from} => #{task_to}): #{e.message}", e.backtrace
rescue BaseException => e
@error_handler.register_resolution_failures_from_exception(
[task], e, e.message
)
end
end

def validate_abstract_network
def validate_abstract_network(error_handler: @error_handler)
super

if Syskit.conf.transformer_enabled?
Expand All @@ -59,12 +70,15 @@ def validate_abstract_network
transformer_tasks = plan.find_local_tasks(Syskit::TaskContext).
find_all { |task| task.model.transformer }
transformer_tasks.each do |task|
SystemNetworkGeneratorExtension.validate_frame_selection_consistency_through_inputs(task)
SystemNetworkGeneratorExtension
.validate_frame_selection_consistency_through_inputs(
task, error_handler
)
end
end
end

def self.validate_frame_selection_consistency_through_inputs(task)
def self.validate_frame_selection_consistency_through_inputs(task, error_handler)
task.each_annotated_port do |task_port, task_frame|
next if !task_port.input? || !task_frame
task_port.each_frame_of_connected_ports do |other_port, other_frame|
Expand All @@ -76,6 +90,10 @@ def self.validate_frame_selection_consistency_through_inputs(task)
other_frame)
end
end
rescue FrameSelectionConflict => e
error_handler.register_resolution_failures_from_exception(
[task], e, e.message
)
end
task.each_transform_port do |task_port, task_transform|
next if !task_port.input?
Expand All @@ -90,6 +108,10 @@ def self.validate_frame_selection_consistency_through_inputs(task)
task_transform.to, other_transform.to)
end
end
rescue FrameSelectionConflict => e
error_handler.register_resolution_failures_from_exception(
[task], e, e.message
)
end
end
end
Expand Down
68 changes: 15 additions & 53 deletions ruby/lib/transformer/syskit/frame_propagation.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,40 +224,6 @@ def initial_information(task)
end
end

class PortAssociationMismatch < RuntimeError
# The problematic endpoint, as a [task, port_name] pair
attr_reader :endpoint
# The other side of the problematic connection(s)
attr_reader :connections
# The association type expected by +endpoint+. Can either be 'frame'
# for an association between a port and a frame, and 'transform' for
# an association between a port and a transformation.
attr_reader :association_type

def initialize(task, port_name, type)
@endpoint = [task, port_name]
@association_type = type

@connections = []
task.each_concrete_input_connection(port_name) do |source_task, source_port_name, _|
@connections << [source_task, source_port_name]
end
task.each_concrete_output_connection(port_name) do |_, sink_port_name, sink_task, _|
@connections << [sink_task, sink_port_name]
end
end

def pretty_print(pp)
pp.text "#{endpoint[0]}.#{endpoint[1]} was expecting an association with a #{association_type}, but one or more connections mismatch"
pp.nest(2) do
pp.breakable
pp.seplist(connections) do |conn|
pp.text "#{conn[0]}.#{conn[1]}"
end
end
end
end

def propagate_task(task)
return if !(tr = task.model.transformer)

Expand Down Expand Up @@ -295,7 +261,7 @@ def propagate_task(task)
next if has_final_information_for_port?(task, port.name)

if selected_frame = task.selected_frames[frame_name]
if !has_information_for_port?(task, port.name)
unless has_information_for_port?(task, port.name)
add_port_info(task, port.name, FrameAnnotation.new(task, frame_name, selected_frame))
done_port_info(task, port.name)
end
Expand All @@ -305,6 +271,7 @@ def propagate_task(task)
end
tr.each_transform_port do |port, transform|
next if has_final_information_for_port?(task, port.name)

from = task.selected_frames[transform.from]
to = task.selected_frames[transform.to]
add_port_info(task, port.name, TransformAnnotation.new(task, transform.from, from, transform.to, to))
Expand Down Expand Up @@ -332,19 +299,8 @@ def propagate_task(task)
# frame to a global frame name
def self.initial_frame_selection_from_device(task, dev)
tr = task.model.transformer
if selected_frame = dev.frame
if !task.transformer.has_frame?(selected_frame)
raise Transformer::InvalidConfiguration, "undefined frame #{selected_frame} selected as reference frame for #{dev}, known frames: #{task.transformer.frames.sort.join(", ")}"
end
end
if selected_transform = dev.frame_transform
if !task.transformer.has_frame?(selected_transform.from)
raise Transformer::InvalidConfiguration, "undefined frame #{selected_transform.from} selected as 'from' frame for #{dev}, known frames: #{task.transformer.frames.sort.join(", ")}"
end
if !task.transformer.has_frame?(selected_transform.to)
raise Transformer::InvalidConfiguration, "undefined frame #{selected_transform.to} selected as 'to' frame for #{dev}, known frames: #{task.transformer.frames.sort.join(", ")}"
end
end
selected_frame = dev.frame
selected_transform = dev.frame_transform

new_selections = Hash.new
task.find_all_driver_services_for(dev).each do |srv|
Expand Down Expand Up @@ -427,15 +383,21 @@ def self.initialize_selected_frames(task, current_selection)
current_selection
else
debug { "adding frame selection from #{task}: #{new_selections}" }
new_selections.each do |frame_name, selected_frame|
if !task.transformer.has_frame?(selected_frame)
raise InvalidConfiguration, "undefined frame #{selected_frame} selected for '#{frame_name}' in #{task}, known frames: #{task.transformer.frames.sort.join(", ")}"
end
end
new_selections
end
result = result.merge(static_frames)
task.select_frames(result)

# Select the frames and then validate that they exist. This is using the fact
# that any raised exception is caught and resolved independently.
result.each do |frame_name, selected_frame|
unless task.transformer.has_frame?(selected_frame)
raise InvalidConfiguration,
"undefined frame #{selected_frame} selected for "\
"'#{frame_name}' in #{task}, known frames: " \
"#{task.transformer.frames.sort.join(", ")}"
end
end
result
end

Expand Down
53 changes: 39 additions & 14 deletions ruby/lib/transformer/syskit/plugin.rb
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Transformer
module SyskitPlugin
class MissingTransform < Syskit::Component
Expand Down Expand Up @@ -88,7 +90,6 @@ def self.compute_required_transformations(manager, task)
)
end


static.each do |trsf|
static_transforms[[trsf.from, trsf.to]] = trsf
end
Expand All @@ -99,10 +100,11 @@ def self.compute_required_transformations(manager, task)
# Just ignore it here, we don't need to instanciate it
# ourselves
next if dyn.producer.kind_of?(Orocos::Spec::Port)

dynamic_transforms[dyn.producer] << dyn
end
end
return static_transforms, dynamic_transforms
[static_transforms, dynamic_transforms]
end

def self.instanciate_producer(manager, task, producer_model, transformations)
Expand Down Expand Up @@ -166,7 +168,7 @@ def self.instanciate_producer(manager, task, producer_model, transformations)
#
# @return [Boolean] true if producers have been added to the plan and
# false otherwise
def self.add_needed_producers(tasks, instanciated_producers)
def self.add_needed_producers(tasks, instanciated_producers, engine)
has_new_producers = false
tasks.each do |task|
dependency_graph = task.relation_graph_for(Roby::TaskStructure::Dependency)
Expand All @@ -179,7 +181,7 @@ def self.add_needed_producers(tasks, instanciated_producers)
task.static_transforms = static_transforms.values
dynamic_transforms.each do |producer_model, transformations|
producer_tasks = instanciated_producers[producer_model]
if !producer_tasks.empty?
unless producer_tasks.empty?
is_recursive = producer_tasks.any? do |prod_task|
prod_task == task || dependency_graph.reachable?(prod_task, task)
end
Expand All @@ -188,11 +190,17 @@ def self.add_needed_producers(tasks, instanciated_producers)
end
end

if producer_task = instanciate_producer(tr_manager, task, producer_model, transformations)
producer_task = instanciate_producer(
tr_manager, task, producer_model, transformations
)
if producer_task
has_new_producers = true
instanciated_producers[producer_model] << producer_task
end
end
rescue BaseException => e
engine.error_handler
.register_resolution_failures_from_exception([task], e, e.message)
end
has_new_producers
end
Expand Down Expand Up @@ -250,6 +258,11 @@ def self.instanciation_postprocessing_hook(engine, plan)
tasks = plan.find_local_tasks(Syskit::Component).roots(Roby::TaskStructure::Dependency)
tasks.each do |root_task|
propagate_local_transformer_configuration(root_task)
rescue BaseException => e
engine.error_handler
.register_resolution_failures_from_exception(
[root_task], e, e.message
)
end
end

Expand Down Expand Up @@ -282,15 +295,28 @@ def self.propagate_local_transformer_configuration(root_task)
# It only checks its inputs, as it is meant to iterate over all tasks
def self.instanciated_network_postprocessing_hook(engine, plan)
needed = true
all_producers = Hash.new { |h, k| h[k] = Array.new }
all_producers = Hash.new { |h, k| h[k] = [] }
while needed
FramePropagation.compute_frames(plan)
transformer_tasks = plan.find_local_tasks(Syskit::TaskContext).
find_all { |task| task.model.transformer }

# Now find out the frame producers that each task needs, and add them to
# the graph
needed = add_needed_producers(transformer_tasks, all_producers)
begin
FramePropagation.compute_frames(plan)
transformer_tasks = plan.find_local_tasks(Syskit::TaskContext)
.find_all { |task| task.model.transformer }

# Now find out the frame producers that each task needs, and add them
# to the graph
needed =
add_needed_producers(transformer_tasks, all_producers, engine)
rescue PortAssociationMismatch => e
engine.error_handler
.register_resolution_failures_from_exception(
e.endpoint.first, e, e.message
)
rescue FrameSelectionConflict, StaticFrameChangeError => e
engine.error_handler
.register_resolution_failures_from_exception(
e.task, e, e.message
)
end
end
end

Expand Down Expand Up @@ -404,4 +430,3 @@ def self.register
end
end
end

Loading

0 comments on commit cafa22a

Please sign in to comment.