Skip to content

Commit

Permalink
style and warning fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
doudou committed May 28, 2018
1 parent 8d27bbc commit 541f648
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 71 deletions.
7 changes: 3 additions & 4 deletions lib/roby/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,6 @@ def created_log_dir?
def log_save_metadata(append: true)
path = File.join(log_dir, 'info.yml')

info = Array.new
current =
if File.file?(path)
YAML.load(File.read(path)) || Array.new
Expand Down Expand Up @@ -1712,7 +1711,7 @@ def require_planners
end

def auto_require_planners
search_path = self.auto_load_search_path
self.auto_load_search_path

prefixes = ['actions']
if backward_compatible_naming?
Expand Down Expand Up @@ -1941,7 +1940,7 @@ def run(thread_priority: 0, &block)
def join
@thread.join

rescue Exception => e
rescue Exception
if @thread.alive? && execution_engine.running?
if execution_engine.forced_exit?
raise
Expand All @@ -1963,7 +1962,7 @@ def running?
#
# Simply execs the same command line
def restart!
Kernel.exec *@restart_cmdline
Kernel.exec(*@restart_cmdline)
end

# Indicates to whomever is managing this app that {#restart} should be
Expand Down
8 changes: 3 additions & 5 deletions lib/roby/coordination/models/action_state_machine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ module Models
# stand = state move(speed: 0)
# # This monitor triggers each time the system moves more than
# # 0.1 meters
# d_monitor = task monitor_movement_threshold(d: 0.1)
# d_monitor = task monitor_movement_threshold(d: 0.1)
# # This monitor triggers after 20 seconds
# t_monitor = task monitor_time_threshold(t: 20)
# t_monitor = task monitor_time_threshold(t: 20)
# # Make the distance monitor run in the move state
# move.depends_on d_monitor
# # Make the time monitor run in the stand state
Expand Down Expand Up @@ -127,7 +127,7 @@ def capture(state, event = nil, &block)
if block
lambda(&block)
else
lambda { |event| event.context.first }
lambda { |ev| ev.context.first }
end

capture = Capture.new(filter)
Expand Down Expand Up @@ -224,5 +224,3 @@ def to_s
end
end
end


9 changes: 3 additions & 6 deletions lib/roby/coordination/task_script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def resolve_event(event)
raise ArgumentError, "#{model.root} has no event called #{symbol}"
end
end
script_task, model_task = resolve_task(event.task)
_, model_task = resolve_task(event.task)
model_event = model_task.find_event(event.symbol)
script_event = instance_for(model_event)
return script_event, model_event
Expand Down Expand Up @@ -191,7 +191,7 @@ def transition!
# emit an event (and quit the block)
#
# @param [Hash] options
# @option options [Event] :event (nil) if set, the given event will
# @option options [Event] :emit (nil) if set, the given event will
# be emitted when the timeout is reached. Otherwise, a
# Script::TimedOut exception is generated with the script's
# supporting task as origin
Expand All @@ -207,7 +207,7 @@ def timeout(seconds, options = Hash.new, &block)
def timeout_start(seconds, options = Hash.new)
options, timeout_options = Kernel.filter_options options, emit: nil
if event = options[:emit]
script_event, model_event = resolve_event(event)
_, model_event = resolve_event(event)
end
model.timeout_start(seconds, timeout_options.merge(emit: model_event))
end
Expand Down Expand Up @@ -288,6 +288,3 @@ def transition!
end
end
end



7 changes: 3 additions & 4 deletions lib/roby/execution_engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,6 @@ def gather_propagation(initial_set = Hash.new)
@propagation_sources = nil
@propagation_step_id = 0

before = @propagation
propagation_context([]) do
yield
end
Expand Down Expand Up @@ -855,7 +854,7 @@ def error_handling_phase(events_errors)

# Compute the set of unhandled fatal exceptions
def compute_kill_tasks_for_unhandled_fatal_errors(fatal_errors)
kill_tasks = fatal_errors.inject(Set.new) do |tasks, (exception, affected_tasks)|
kill_tasks = fatal_errors.inject(Set.new) do |tasks, (_exception, affected_tasks)|
tasks.merge(affected_tasks)
end
# Tasks might have been finalized during exception handling, filter
Expand Down Expand Up @@ -1052,7 +1051,7 @@ def prepare_propagation(target, is_forward, info)
# the forwardings and signals that the propagation of the considered event
# have added.
def event_propagation_step(current_step, propagation_info)
signalled, step_id, forward_info, call_info = next_event(current_step)
signalled, _step_id, forward_info, call_info = next_event(current_step)

next_step = nil
if !call_info.empty?
Expand Down Expand Up @@ -2440,7 +2439,7 @@ def execute(catch: [], type: :external_events)
when :ret
return value
when :throw
throw *value
throw(*value)
else
raise value
end
Expand Down
10 changes: 7 additions & 3 deletions lib/roby/interface/rest/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ class Server
# @param [Integer] port the port the server should bind to if it
# is a TCP server. Set to zero to auto-allocate. Ignored if 'host'
# is the path to a UNIX socket.
def initialize(app, host: '0.0.0.0', port: Roby::Interface::DEFAULT_REST_PORT,
api: REST::API)
def initialize(app,
host: '0.0.0.0',
port: Roby::Interface::DEFAULT_REST_PORT,
api: REST::API, **thin_options)

@app = app
@host = host
@interface = Interface.new(app)
Expand All @@ -42,7 +45,8 @@ def initialize(app, host: '0.0.0.0', port: Roby::Interface::DEFAULT_REST_PORT,
run api
end
end
@server = Thin::Server.new(host, port, rack_app, signals: false)
@server = Thin::Server.new(host, port, rack_app,
signals: false, **thin_options)
@server.silent = true
if @server.backend.respond_to?(:port)
@original_port = port
Expand Down
21 changes: 10 additions & 11 deletions lib/roby/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Plan < DistributedObject
extend Logger::Hierarchy
extend Logger::Forward
include DRoby::EventLogging

# The Peer ID of the local owner (i.e. of the local process / execution
# engine)
attr_accessor :local_owner
Expand Down Expand Up @@ -219,7 +219,7 @@ def merge_base(plan)
task_index.merge(plan.task_index)
task_events.merge(plan.task_events)
end

def merge_relation_graphs(plan)
# Now merge the relation graphs
#
Expand Down Expand Up @@ -607,7 +607,7 @@ def add_permanent_event(event)

# True if the given event is registered as a permanent event on self
def permanent_event?(generator)
@permanent_events.include?(generator)
@permanent_events.include?(generator)
end

# Removes a task from the set of permanent tasks
Expand Down Expand Up @@ -677,7 +677,7 @@ def handle_force_replace(from, to)
elsif to.plan != self
raise ArgumentError, "trying to replace #{to} but its plan is #{to.plan}, expected #{self}"
elsif from == to
return
return
end

# Swap the subplans of +from+ and +to+
Expand Down Expand Up @@ -758,7 +758,7 @@ def exclude_tasks(tasks)
@tasks.merge(tasks)
self
end

# Exclude a single task
#
# @param [Task] task the task to be excluded
Expand All @@ -767,7 +767,7 @@ def exclude_task(task)
@tasks << task
self
end

# Tests whether a task is to be excluded
#
# @param [Task] task
Expand Down Expand Up @@ -1378,7 +1378,7 @@ def each_task
return enum_for(__method__) if !block_given?
@tasks.each { |t| yield(t) }
end

# Returns +object+ if object is a plan object from this plan, or if
# it has no plan yet (in which case it is added to the plan first).
# Otherwise, raises ArgumentError.
Expand Down Expand Up @@ -1592,15 +1592,15 @@ def replan(task)
return task.create_fresh_copy
end

planner = replan(old_planner = task.planning_task)
planner = replan(task.planning_task)
planned = task.create_fresh_copy
planned.abstract = true
planned.planned_by planner
replace(task, planned)
planned
end


# The set of blocks that should be called to check the structure of the
# plan.
#
Expand Down Expand Up @@ -1635,7 +1635,7 @@ def self.check_failed_missions(plan)
result
end
structure_checks << method(:check_failed_missions)

# @api private
#
# Normalize the value returned by one of the {#structure_checks}, by
Expand Down Expand Up @@ -1909,4 +1909,3 @@ def each_object_in_transaction_stack(object)
end
end
end

17 changes: 8 additions & 9 deletions lib/roby/task_structure/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def self.merge_fullfilled_model(model, required_models, required_arguments)
end
end

arguments = arguments.merge(required_arguments) do |name, old, new|
arguments = arguments.merge(required_arguments) do |name, old, new|
if old != new
raise Roby::ModelViolation, "inconsistency in fullfilled models: #{old} and #{new}"
end
Expand Down Expand Up @@ -434,7 +434,7 @@ def resolve_role_path(*path)
# +self+.
#
# I.e. if ['role1', 'role2', 'role3'] is a role path from +self+ to
# +task, it means that
# +task, it means that
#
# task1 = self.child_from_role('role1')
# task2 = task1.child_from_role('role2')
Expand Down Expand Up @@ -479,7 +479,7 @@ def role_paths(task, validate = true)
#
# success:: the list of success events. The default is [:success]
# failure:: the list of failing events. The default is [:failed]
# model::
# model::
# a <tt>[task_model, arguments]</tt> pair which defines the task
# model the parent is expecting. The default value is to get these
# parameters from +task+
Expand All @@ -504,9 +504,9 @@ def depends_on(task, options = {})
raise ArgumentError, "cannot add a dependency of a task to itself"
end

options = Dependency.validate_options options,
model: [task.provided_models, task.meaningful_arguments],
success: :success.to_unbound_task_predicate,
options = Dependency.validate_options options,
model: [task.provided_models, task.meaningful_arguments],
success: :success.to_unbound_task_predicate,
failure: false.to_unbound_task_predicate,
remove_when_done: true,
consider_in_pending: true,
Expand Down Expand Up @@ -639,7 +639,7 @@ def first_children
# However, this fails in case +self+ is a root task in the dependency
# relation. Moreover, it might be handy to over-constrain the model
# computed through the dependency relation.
#
#
# In both cases, a model can be specified explicitely by setting the
# fullfilled_model attribute. The value has to be
#
Expand Down Expand Up @@ -826,7 +826,7 @@ def implicit_fullfilled_model
def fullfilled_model
explicit_fullfilled_model || implicit_fullfilled_model
end

# Enumerates the models that all instances of this task model fullfill
#
# @yieldparam [Model<Task>,Model<TaskService>] model
Expand Down Expand Up @@ -919,4 +919,3 @@ def involved_plan_object?(obj)
end
end
end

13 changes: 6 additions & 7 deletions test/coordination/models/test_action_state_machine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def start_task(task)
it "raises if attempting to specify a state that is not a toplevel state" do
monitoring = state_machine.state(action_m.monitoring_task)
start.depends_on(monitoring)

assert_raises(Roby::Coordination::Models::NotToplevelState) do
state_machine.forward monitoring, monitoring.success_event,
state_machine.success_event
Expand All @@ -161,7 +161,7 @@ def start_task(task)
it "raises if attempting to specify an event that is not active in the state" do
monitoring = state_machine.state(action_m.monitoring_task)
state_machine.transition start.success_event, monitoring

assert_raises(Roby::Coordination::Models::EventNotActiveInState) do
state_machine.forward start, monitoring.success_event,
state_machine.success_event
Expand Down Expand Up @@ -206,7 +206,7 @@ def start_task(task)
describe "#state" do
it "can use any object that responds to #to_action_state" do
obj = flexmock
obj.should_receive(:to_action_state).and_return(task = Roby::Task.new)
obj.should_receive(:to_action_state).and_return(Roby::Task.new)
task_m = self.task_m
assert_raises(ArgumentError) do
Roby::Actions::Interface.new_submodel do
Expand Down Expand Up @@ -292,10 +292,9 @@ def start_task(task)
coordination_model

assert_equal 1, rebound.forwards.size
source_event, target_event = rebound.forwards.first

assert_equal [[new_action_m.monitoring_task, 'task_dependency']],
rebound.find_state_by_name('start_task').dependencies.map { |task, role| [task.action, role] }
rebound.find_state_by_name('start_task').dependencies.
map { |task, role| [task.action, role] }
end
end

Expand Down Expand Up @@ -352,7 +351,7 @@ def start_task(task)
# task model can be transferred afterwards
test_task_m = Roby::Task.new_submodel
start_task_m = Roby::Task.new_submodel
description = action_m.describe("with_arguments").
action_m.describe("with_arguments").
optional_arg('test', 'test', test_task_m)
action_m.action_state_machine 'with_arguments' do
start state(start_task_m)
Expand Down
7 changes: 3 additions & 4 deletions test/coordination/test_action_script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ def start_script
end

it "fails if asked to wait for an unreachable event" do
script = start_script
start_script
expect_execution { action_task.intermediate_event.unreachable! }.
to { have_error_matching Models::Script::DeadInstruction.match.with_origin(root_task) }
end

it "fails if the event it is waiting for becomes unreachable" do
task_model.event :second
script_model.wait script_task.second_event
script = start_script
start_script
execute { action_task.second_event.unreachable! }
expect_execution { action_task.intermediate_event.unreachable! }.
to { have_error_matching Models::Script::DeadInstruction.match.with_origin(root_task) }
Expand Down Expand Up @@ -135,7 +135,7 @@ def start_script
and_return(action_task = task_model.new)
script_model.forward script_task.intermediate_event, script_model.success_event
script_model.start script_task
script = script_model.new(root_task)
script_model.new(root_task)
execute { root_task.start! }
expect_execution do
action_task.start!
Expand All @@ -148,4 +148,3 @@ def start_script
end
end
end

Loading

0 comments on commit 541f648

Please sign in to comment.