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

fix default event port behavior along with FileDescriptorActivity #1

Merged
merged 2 commits into from
May 6, 2020

Conversation

doudou
Copy link
Member

@doudou doudou commented May 5, 2020

This is the rock-specific version of orocos-toolchain#320

For the rationale behind the existence of this fork, see rock-core/package_set#167

Depends on:

With the introduction of the work() method, which allowed to
be more precise on what gets called in which condition,
an event port under FileDescriptorActivity would not make
updateHook get called anymore.

This broke the Rock workflow with file descriptor activities.
(Un)fortunately, the effect has been subtle enough to not be
noticed readily. Or people started deploying tasks under the
normal Activity where before it would work with the FDA (I'm
guilty of that).

This is a rather convoluted code path (the characters in the
play are a FileDescriptorActivity 'FDA', ExecutionEngine 'EE'
and a TaskContext 'TC'

  • the port signal calls FDA::trigger()
  • FDA::trigger() wakes-up the FDA loop, which calls EE::work(Trigger)
  • EE::work(Trigger) calls EE::processPortCallbacks but NOT
    EE::processHooks (expected from the work() refactoring)

At that point, we have to remember that TC::addEventPort registers
a port callback that by default calls TC::trigger(). The
comment at that point in the code itself says "default schedules
an updateHook" (which we're indeed expecting with a FDA)

  • so, EE::processPortCallbacks in the end calls TC::trigger
  • which, by default calls FDA::timeout

And FDA::timeout() was not implemented.

@doudou doudou requested review from 2maz and g-arjones May 5, 2020 00:26
rtt/extras/FileDescriptorActivity.cpp Outdated Show resolved Hide resolved
With the introduction of the work() method, which allowed to
be more precise on what gets called in which condition,
an event port under FileDescriptorActivity would not make
updateHook get called anymore.

This broke the Rock workflow with file descriptor activities.
(Un)fortunately, the effect has been subtle enough to not be
noticed readily. Or people started deploying tasks under the
normal Activity where before it would work with the FDA (I'm
guilty of that).

This is a rather convoluted code path (the characters in the
play are a FileDescriptorActivity 'FDA', ExecutionEngine 'EE'
and a TaskContext 'TC'

- the port signal calls FDA::trigger()
- FDA::trigger() wakes-up the FDA loop, which calls EE::work(Trigger)
- EE::work(Trigger) calls EE::processPortCallbacks but NOT
  EE::processHooks (expected from the work() refactoring)

At that point, we have to remember that TC::addEventPort registers
a port callback that by default calls TC::trigger(). The
comment at that point in the code itself says "default schedules
an updateHook" (which we're indeed expecting with a FDA)

- so, EE::processPortCallbacks in the end calls TC::trigger
- which, by default calls FDA::timeout

And FDA::timeout() was not implemented.
@doudou doudou merged commit 80ff0ea into master May 6, 2020
@doudou doudou deleted the fix_event_port_and_fda branch May 7, 2020 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants