For more information about this bug, visit
A comment was added:
------- Comment #12 from peter [dot] soetens [..] ... 2007-03-13 12:44

Created an attachment (id=88)
--> (http://www.fmtc.be/orocos-bugzilla/attachment.cgi?id=88&action=view)
Do not accumulate trigger() events

This patch fixes the case where several sources call trigger() (or start()) on
a non periodic activity (or thread), accumulating 'events'. The current code
keeps executing loop() even after stop() and finalize() are called, until the
count reaches zero. The correct behaviour is to discard all events after
stop().

This patch also changes the response of trigger() from 'counting' to 'binary'.
This breaks applications that assume that the loop() function is executed as
many times as trigger() is executed. This can in trun be avoided by removing
the
if ( rtos_sem_value(&sem) > 0 )
return false;
statement from start().

I'm not sure if the last part belongs in the patch.

For more information about this bug, visit
A comment was added:
------- Comment #11 from peter [dot] soetens [..] ... 2007-03-05 09:22

(In reply to comment #10)
> The code example is _not_ a clean way to solve the problem! It's a hack that
> one can only understand if one knows how the RTT support works: nothing in this
> code snippet tells me anything about how the code will behave...

But the API documentation will. This is the picture: The user writes an
update() function. If the component is run by a periodic activity, update()
will be periodically called when the activity is started. If the component is
run by a non-periodic activity, update is called once after start() and then
each time a 'trigger()' (call it an event, or what you like) occurs. What the
user does in this code:

void update()
{
// ... here comes my algorithm...
//....

// if some condition is met:
this->getActivity()->trigger();
}

He just 'fires off' a new event to the non periodic activity that it may be run
again. Since this happens _inside_ update, this will cause the thread to call
update() immediately again when update() returns (the thread does not know that
update() trigger()'ed it anyway).

Peter

For more information about this bug, visit
A comment was added:
------- Comment #10 from herman [dot] bruyninckx [..] ... 2007-03-02 19:17

The code example is _not_ a clean way to solve the problem! It's a hack that
one can only understand if one knows how the RTT support works: nothing in this
code snippet tells me anything about how the code will behave...

For more information about this bug, visit
A comment was added:
------- Comment #9 from wim [dot] meeussen [..] ... 2007-03-02 19:07

> void update() {
> // do blocking stuff
> if (another_run_required)
> this->getActivity()->trigger();
> }
>
> Because you call trigger(), when update is left, the thread will re-enter
> directly the ExecutionEngine, which will call update() again etc.
>
> This is a quite clean way of solving the issue without breaking Backwards
> Compatibility. It even allows the 'breakLoop()' function to work correctly,
> such that the thread can be stop()'ed at any time.

Do you mean that in this way I can set the "another_run_required" to false in
the shutdown function of the taskcontext? That's a nice solution!

Wim

For more information about this bug, visit
A comment was added:
------- Comment #8 from peter [dot] soetens [..] ... 2007-03-02 16:07

(In reply to comment #5)
> the "update" function of the taskcontext. In the current implementation a
> nonperiodic activity calls update once when the user executes a command. Should
> we change this behavior so it calls update over and over again, but
> non-periodically?

Unfortunately, this 'change of behaviour' is unacceptable for RTT 1.2.0. I
agree that the current behaviour is 'unusable' in many cases and the new
proposed behaviour is far better suited for doing blocking communication,
nevertheless we are not allowed to change this in the 1.x series.

There is another 'work-around' though. Write your update function like this:

void update() {
// do blocking stuff
if (another_run_required)
this->getActivity()->trigger();
}

Because you call trigger(), when update is left, the thread will re-enter
directly the ExecutionEngine, which will call update() again etc.

This is a quite clean way of solving the issue without breaking Backwards
Compatibility. It even allows the 'breakLoop()' function to work correctly,
such that the thread can be stop()'ed at any time.

For more information about this bug, visit
A comment was added:
------- Comment #7 from peter [dot] soetens [..] ... 2007-03-02 16:00

(In reply to comment #6)
> "nonperiodic" is a _negative name_: it says what the thing is _not_, and that's
> a bad policy with giving names...

I agree. LoopingActivity is for that matter a better choice.

>
> What's wrong with an event handler? All this infrastructure _is_ already
> available in Orocos.

Since someone needs to emit the event. Now if your hardware does not support
that, 'someone' is polling it anyway and emits then an event if the new data is
there. Well, that is exactly what Wim is proposing, but he wants to standardise
the the 'polling' part.

>
> I would certainly not add a "nonperiodic activity" that would call its update
> all the time! You can do this silly stuff in a while loop in any other activity
> already :-)

You can't. Doing so in a PeriodicActivity would cause overruns which may
emergency stop your thread. Hence you need an Activity type that explicitly
allows the user code to go 'out for lunch'.

For more information about this bug, visit
A comment was added:
------- Comment #6 from herman [dot] bruyninckx [..] ... 2007-03-02 15:46

"nonperiodic" is a _negative name_: it says what the thing is _not_, and that's
a bad policy with giving names...

What's wrong with an event handler? All this infrastructure _is_ already
available in Orocos.

I would certainly not add a "nonperiodic activity" that would call its update
all the time! You can do this silly stuff in a while loop in any other activity
already :-)

For more information about this bug, visit
A comment was added:
------- Comment #5 from wim [dot] meeussen [..] ... 2007-03-02 15:40

> you're right, but this mapping from polling to Orocos event should best be
> done _outside_ of Orocos... The concrete solution will be platform-dependent
> anyway, using very RTOS specific code. I would prefer to add only platform
> independent code into orocos components!

Orocos should offer the component builder the option to build a platform
dependent component that reads for example the serial port. Maybe the real
problem here is not that we need a new type of activity, but the way the
nonperiodic activity should behave. What I called loopactivity is actually the
nonperiodic activity. The question is what the nonperiodic activity does with
the "update" function of the taskcontext. In the current implementation a
nonperiodic activity calls update once when the user executes a command. Should
we change this behavior so it calls update over and over again, but
non-periodically?

Wim

For more information about this bug, visit
A comment was added:
------- Comment #4 from herman [dot] bruyninckx [..] ... 2007-03-02 15:25

you're right, but this mapping from polling to Orocos event should best be done
_outside_ of Orocos... The concrete solution will be platform-dependent anyway,
using very RTOS specific code. I would prefer to add only platform independent
code into orocos components!

Of course, the Orocos source tree has (preliminary) support for these
platform-dependent 'driver issues'...

For more information about this bug, visit
A comment was added:
------- Comment #3 from wim [dot] meeussen [..] ... 2007-03-02 15:09

> You are re-inventing event handling....! Your laser scan driver should fire
> an event when a new measurement is available; your laser scan sensor
> processor should be event driven.

But the laser scan drive does not fire an event. We have the same issue with
the krypton camera system, where we wait for packages that arrive on the
realtime ethernet link. Many drivers do not implement an event system but a
waiting system. One of the reasons why we wrap our hardware in Orocos
components, is because when we build an applications, we don't want to deal
with the specifics on how to access the hardware. When the laserscanner is
embedded in an Orocos components, we only need to read its ports to get the
latest scan. It is the laser scanner component that should fire an event to
notify other components when a new measurement arrived. But internally the
laserscanner component should use the hardware driver as it is; in our case
this is waiting for a measurement to arrive.

For more information about this bug, visit
A comment was added:
------- Comment #2 from herman [dot] bruyninckx [..] ... 2007-03-02 13:55

You are re-inventing event handling....! Your laser scan driver should fire an
event when a new measurement is available; your laser scan sensor processor
should be event driven. Please _don't_ introduce another 'activity', especially
with the meaningless and semantically confusing name of "looping" in it...

BTW The same event-driven data flow holds for most applications, such as the
visual servoing one. Maybe you could get inspiration there? And then notice
that that OCL thing is making some mistakes too, which you should then correct
together :-)

For more information about this bug, visit

peter [dot] soetens [..] ... changed:

What |Removed |Added
---------------------------------------------------------------------------
Target Milestone|--- |1.2.0

------- Comment #1 from peter [dot] soetens [..] ... 2007-03-02 12:59

(In reply to comment #0)
> A solution might be to create a new type of activity: loopingActivity. This
> type of activity would call the update function again and again, but not with a
> fixed period, but every time the previous run of the update function was
> finished. In this way I can block on the serial port in the update function,
> and when a measurement is available, receive the measurement. But the update
> would not contain a loop any more. In between two update runs, the activity can
> check for queued commands and execute them.

You're full of bright ideas lately :-) This seems a very common use case for
components that communicate with hardware (or a network socket,...). I guess
the implementation of LoopingActivity will be based on NonPeriodicActivity, as
it will create a dedicated SingleThread behind the scenes. I took a quick look
at the ExecutionEngine and *Processor classes, and I believe no existing code
needs to be changed. Just copy over NonPeriodicActivity and invoke step() in
loop() in a while loop, and make sure the stop() function still works. Also
disable the trigger() method (return false;).