rock-core / tools-roby

The roby plan manager
Other
3 stars 11 forks source link

Emission handling when using monitors #3

Open goldhoorn opened 10 years ago

goldhoorn commented 10 years ago

I dont't want to continue the discussion on stackexchange. I uploaded the logfile to:

http://auv.informatik.uni-bremen.de/framework/logs/syskit-event-submission-bug/avalon-events.log

The error is between cycle 3457 and 3610, in 3457 the PipelineDetector goes from stop to success, in the next cycle it emits a weak_signal. I assume the problem is a race-problem. Syskit needs to long for the reconfiguration of the network, the task therefore is not stopped and changes his state. In the meantime syskit assumes that the task is stopped and can't handle the event submission anymore.

But in general what i wonder, the task should not be stopped, because it is needed in the later setup also. The next state of the state-machine should reuse it.. but i think this should be resolved by the graph-merging in later steps that could not archived caused by the error.

doudou commented 10 years ago

Hey. Please add screenshots of the steps you are talking about in the issue, thanks.

2014-07-01 11:32 GMT+02:00 Matthias Goldhoorn notifications@github.com:

I dont't want to continue the discussion on stackexchange. I uploaded the logfile to:

http://auv.informatik.uni-bremen.de/framework/logs/syskit-event-submission-bug/avalon-events.log

The error is between cycle 3457 and 3610, in 3457 the PipelineDetector goes from stop to success, in the next cycle it emits a weak_signal. I assume the problem is a race-problem. Syskit needs to long for the reconfiguration of the network, the task therefore is not stopped and changes his state. In the meantime syskit assumes that the task is stopped and can't handle the event submission anymore.

But in general what i wonder, the task should not be stopped, because it is needed in the later setup also. The next state of the state-machine should reuse it.. but i think this should be resolved by the graph-merging in later steps that could not archived caused by the error.

— Reply to this email directly or view it on GitHub https://github.com/rock-core/tools-roby/issues/3.

goldhoorn commented 10 years ago

Here they are

3457: 3457

3610: 3610

doudou commented 10 years ago

I assume that the one at the top is 3610 and the one at the bottom 3457... Would have been nice to have them in chronological order ...

goldhoorn commented 10 years ago

Sorry, yep, the information was not displayed default, i added the info above. By "editing" the message you would have seen the names...

doudou commented 10 years ago

FYI, there's a nice preview mode to make sure that you did not get the markdown wrong

2014-07-02 9:41 GMT+02:00 Matthias Goldhoorn notifications@github.com:

Sorry, yep, the information was not displayed default, i added the info above. By "editing" the message you would have seen the names...

— Reply to this email directly or view it on GitHub https://github.com/rock-core/tools-roby/issues/3#issuecomment-47745914.

doudou commented 10 years ago

OK ... so.

It is what I thought. Not a race condition, syskit/roby do what they are supposed to do here. It is just that the task emits the weak_signal event, which gets forwarded to the compositions that are not yet running. There are number of reasons why, in this situation, the compositions would not be running (e.g. having a temporal constraint forbidding them to be), so everything in the code behaves as it is supposed to.

Which brings me to the real problem. Theoretically, the forwarding relation should not be setup until the target task is started in this case. However, this is roby-theoretical-from-2007 before syskit started existing and I am now thinking that it should not be the case. IMO, the right fix would be that the forwarding relation should simply be ignored if the target task is not started.

goldhoorn commented 10 years ago

Could you propose a pull-request which i can try?

doudou commented 10 years ago

Nope, because I don't have the time. Feel free to do it, though.

2014-07-04 10:14 GMT+02:00 Matthias Goldhoorn notifications@github.com:

Could you propose a pull-request which i can try?

— Reply to this email directly or view it on GitHub https://github.com/rock-core/tools-roby/issues/3#issuecomment-48018818.

goldhoorn commented 10 years ago

Could you give a hit instead for the right solution instead simply removing the error message?

doudou commented 10 years ago

???

goldhoorn commented 10 years ago

Whats unclear?

I want a hint for the right fix if possible. So that i can provide a correct fix.

Br

Von meinem iPhone gesendet

Am 08.08.2014 um 19:30 schrieb Sylvain Joyeux notifications@github.com:

???

— Reply to this email directly or view it on GitHub.

doudou commented 10 years ago

Ah ... now it's clear ;-)

As I mentioned in https://github.com/rock-core/tools-roby/pull/7, that would require a level of knowledge of Roby's internals that you really don't have... And I have little time to explain it. I am testing different ways to do it, but that's going to take some time. If the original fix works for you, stick to it for now.