Arbitrator bug

[trandi]

Hi everybody,

I've just discovered what I think is a small bug in the Arbitrator class:

line 131 :
int active = _active;// local copy: avoid out of bounds error in 134
i f (active != NONE && _highestPriority > active)
{

It should be "_highestPriority != active" !

It doesn't matter the priority of the new active behavious, as long as it changes, we HAVE to suppress the old one.

thanks,
dan
http://trandi.wordpress.com


P.S. before you ask the question or point it out, YES I did search for a way to raise a ticket for this, but couldn't find... also, does can anybody tell me what one needs to do to be able to check in a fix directly ?

[gloomyandy]

Hmm interesting. I guess you argue that for active to >= to the new thread must imply that the active behaviour has decided that it no longer wants to be active (and so presumably has already exited from the action routine), and therefore has no need to be suppressed... But I'm no expert on this code. I'll get the dev who wrote it to take a look at this thread...

Either way, this is the correct place to bring up such issues, and thanks for doing so...

Andy

[trandi]

This is a fair point, you're saying that the suppress() is called ONLY when the control is FORCIBLY taken away from you, NOT when you give it away...

It does make sense, but, for me at least, it feels "safer" and simpler to know that suppress() is ALWAYS called when I give away the control, be it willing or not...

thanks for you quick reply anyhow !
dan
http://trandi.wordpress.com

[gloomyandy]

Hi Dan,
you may well be right, as I said I'm no expert with this and my theory is just that! As I said I'll get the dev that wrote this to take a look and comment...

Thanks again for bringing it up...

Andy

[roger]

Hi Dan,
You wrote

it feels "safer" and simpler to know that suppress() is ALWAYS called when I give away the control, be it willing or not...

A behavior gives up control when it exits the action() method. The normal exit is when action() runs to completion. If suppress is called, action() should exit quickly (even if unwillingly). In either case, you should be sure that the all necessary "clean up" work is done before action() exits. You can't rely on suppress() to do the clean up, because suppress() cannot be called on an inactive behavior, and the active behavior becomes inactive as soon as action () exits.
I hope this is clear.
Roger

[ula.uvula]

Hi,
I want to try leJOS with a child in my neighborhood.
So I read a little bit about leJOS and was impressed from
all the possibilities. A great job by the developers, thanks.

To understand the Behavior(-Arbitrator) pattern, I tried this
on my PC and think I found the same problem/bug as "trandi".
This is never tested on the NXT.

I think, the suppress method is not called for active behavior
and so the behavior doesn't terminate. Specially, the behavior with
the highest prio is not suppressed.

I have (for understanding/learning) written a simple test with similar behaviors.
Each behavior runs 10 seconds (variable duration).

Interval[From, To] Behavior
[00:00:00, 00:00:10] no behavior is active (both takeControl == false)
[00:00:10, 00:00:20] B1 is running
[00:00:20, 00:00:30] B2 is running
[00:00:30, 00:00:40] no behavior is active (both takeControl == false)
[00:00:40, 00:00:50] B1 is running
[00:00:50, 00:01:00] B2 is running
[00:01:00, 00:01:10] no behavior is active (both takeControl == false)
....

If I change in Arbitrator.java the ">" to "!=", it works correct for my understanding
of the pattern.
if (active != NONE && _highestPriority != active)

Can somebody please verify this problem again, or do I misunderstand the pattern?


Uwe / Germany

Code: Select all


package ula.robotics.lejos.examples;

import java.text.SimpleDateFormat;

import lejos.robotics.subsumption.Arbitrator;
import lejos.robotics.subsumption.Behavior;

public class ArbTest {
   private static int behaviors = 0; // reinitalized by init()
   private static int duration = 10;

   private static long now() {
      return System.currentTimeMillis();
   }

   private static long timeSlot() {
      return now() / 1000 % (behaviors * duration) / duration;
   }

   private Behavior createBehavior(final int bId) {
      return new Behavior() {
         private String name = "B" + bId;
         private int id = bId;
         private boolean suppress = true;

         @Override
         public boolean takeControl() {
            boolean result = timeSlot() == id;
//            print("takeCtrl=" + result);
            return result;
         }

         @Override
         public void suppress() {
            print("suppress=true");
            suppress = true;
         }

         @Override
         public void action() {
            print("action started");
            suppress = false;
            while (!suppress)
               Thread.yield();
            print("action stopped");
         }

         private void print(String message) {
            synchronized (ArbTest.class) {
               System.out.format("%s - %-20s - %s", name, message,   new SimpleDateFormat("HH:mm:ss").format(now()));
               System.out.println();
            }
         }
      };
   }

   private void init() {
      Behavior[] bList = new Behavior[] {
            createBehavior(2),
            createBehavior(1),
      };
      
      behaviors = bList.length + 1;
      
      Arbitrator arbitrator = new Arbitrator(bList);
      arbitrator.start();
   }

   public static void main(String[] args) {
      new ArbTest().init();
   }
}




[roger]

Hallo Ula,
The reason your code does not run correctly with the arbitrator is that your action method does not exit normally when the action is complete, that is when the time slot has expired. Consequently, the highest priority action runs forever since it cannot be suppressed by a lower priority action. The loop should be

Code: Select all

while (!suppress && !takeControl())

I do not immediately see the termination condition of you code. Did I miss something?
Roger

[ula.uvula]

Hi Roger,
thanks for your answer. A typical example for "read first, than ask".

I thought the Arbitrator checks the levels (priorities) and send a "suppress" to
the active behavior, wait for the termination and starts a higher (other) behavior.

So I thought: if takeControl of the active behavior turns to "false",
the Arbitrator calls suppress at the active level!
And the active behavior (action method) can react and terminates. But now I see, it's the job of "action"
to react to a suppress call and also decide when to terminate by itself.

Your recommendation:
while (!suppress && !takeControl())
is a good way to reach this behavior.

Or I use the derived Arbitrator class with the "!="

Uwe / Germany