0.7 problem (subsumption bumpercar with ultrasonic)

This is where you talk about the NXJ software itself, installation issues, and programming talk.

Moderators: 99jonathan, roger, imaqine

0.7 problem (subsumption bumpercar with ultrasonic)

Postby dvdboom » Wed Nov 19, 2008 10:50 pm

After downloading and installing 0.7 my robot program was not working anymore. it looks like it's clear in this default example program (but touchsensor replaced by an ultrasonic) to this question: why is this code not working anymore (robot fires one behavior and then stops)? please advice, any help is appreciated.

import lejos.subsumption.*;
import lejos.nxt.*;

public class DriveForward implements Behavior {
public boolean takeControl() {
return true;
}

public void suppress() {
Motor.A.stop();
Motor.B.stop();
}

public void action() {
Motor.A.forward();
Motor.B.forward();
}
}

/*-------------------------------------------------*/

import lejos.subsumption.*;
import lejos.nxt.*;

public class HitWall implements Behavior {
UltrasonicSensor ultrasonicsensor = new UltrasonicSensor(SensorPort.S1);
//TouchSensor touch = new TouchSensor(SensorPort.S2);
boolean prox = false;

public boolean takeControl() {
//return touch.isPressed();

prox = false;
// check range, detect obstacle
if (ultrasonicsensor.getDistance() < 30 ) {
prox = true;
}else{
prox = false;
}
return prox;
}

public void suppress() {
Motor.A.stop();
Motor.B.stop();
}

public void action() {
// Back up:
Motor.A.backward();
Motor.B.backward();
try{Thread.sleep(1000);}catch(Exception e) {}

// Rotate by causing only one wheel to stop:
Motor.A.stop();
try{Thread.sleep(300);}catch(Exception e) {}
Motor.B.stop();
}
}

/*------------------------------------------------------------*/

import lejos.subsumption.*;

public class BumperCar {
public static void main(String [] args) {
Behavior b1 = new DriveForward();
Behavior b2 = new HitWall();
Behavior [] bArray = {b1, b2};
Arbitrator arby = new Arbitrator(bArray);
arby.start();
}
}
dvdboom
New User
 
Posts: 5
Joined: Wed Nov 19, 2008 10:41 pm

Postby gloomyandy » Wed Nov 19, 2008 11:08 pm

Just to check... I assume you have re-compiled and re-linked your program with 0.7? Just to be 100% sure, did you use the command line tools? If all the above is OK perhaps you could help by providing a little more detail...
What happens? What do you expect to happen? I'm not that familiar with the subsumption classes, but perhaps someone who is will be along soon!

Andy
User avatar
gloomyandy
leJOS Team Member
 
Posts: 4174
Joined: Fri Sep 28, 2007 2:06 pm
Location: UK

Postby dvdboom » Wed Nov 19, 2008 11:38 pm

yes, all is recompiled on 0.7
the example program is included with the lejos install and normally just drives and avoids with a bumper. The only replacement i did was to use the ultrasonic sensor. now it just start to drive and keeps on driving and never sees an object. In my original program the "TakeControl" fires, but the "Action" never happens. This used to work on 0.6
dvdboom
New User
 
Posts: 5
Joined: Wed Nov 19, 2008 10:41 pm

Postby gloomyandy » Thu Nov 20, 2008 12:41 am

Hi,
A few things for you to try (I don't have access to my nxt at the moment so can't try any of this)....
1. Please check that your nxt is running the 0.7 firmware (one of the options in the menu should display the version).
2. Can you add code to your test program to display if the take control method ever fires? You may also want to display the ultrasonic sensor value return. This will tell us if the tak control method is being called, if the sensor is working and if it ever fires.
3. Can you also add code to display if the action statements in your classes are being called. In particular if the one that should take avoiding action.
4. If you switch back to the original code (using a touch sensor), what happens then?

Looking at the logs for the subsumption code it looks like there have been some pretty large changes since 0.6. It may be that a bug has been introduced....

Andy
User avatar
gloomyandy
leJOS Team Member
 
Posts: 4174
Joined: Fri Sep 28, 2007 2:06 pm
Location: UK

Postby lawrie » Thu Nov 20, 2008 3:56 pm

Yes, it looks like Roger's new algorithm for the Arbitrator has broken the BumperCar example and your version of it.

I will look into it, but a circumvention that seems to work is to change the line that creates the Arbitrator in BumperCar to:

Code: Select all
Arbitrator arby = new Arbitrator(bArray, false);
lawrie
leJOS Team Member
 
Posts: 929
Joined: Mon Feb 05, 2007 1:27 pm

Postby dvdboom » Thu Nov 20, 2008 5:49 pm

I've tried your workaround for the bug and great, it works again!
thanx a lot.

Let's hope it's fixed in the next release, but I'm helped for now :D
cheers!
dvdboom
New User
 
Posts: 5
Joined: Wed Nov 19, 2008 10:41 pm

Postby lawrie » Thu Nov 20, 2008 6:15 pm

Great that the circumvention works for you. I have started a discussion with Roger on how to fix it properly.

Incidentally, all the code you put in the takeControl method:

Code: Select all
prox = false;
// check range, detect obstacle
if (ultrasonicsensor.getDistance() < 30 ) {
prox = true;
}else{
prox = false;
}
return prox;


could be replaced by:

Code: Select all
return (ultrasonicsensor.getDistance() < 30);
lawrie
leJOS Team Member
 
Posts: 929
Joined: Mon Feb 05, 2007 1:27 pm

Postby joseph.goldstone » Wed Nov 26, 2008 10:03 pm

I too was bit by this incompatible change. Why couldn't the constructor which takes only the array of Behaviors call the two-arg version with a second argument whose default was false, not true? That would have preserved compatibility.

As for deeper compatibility I haven't had a chance to test it extensively.

A couple of comments on the new code:

In the start() method of Arbitrator, what is the purpose of that first while loop, just after the call to monitor.start() ? It seems as if it's going to busy-wait for _highestPriority to be other than NONE, but that's done in the loop immediately below, and when it's done in the loop, there's a Thread.yield(). It seems as if it would be easier on all the other threads to just dive straight into the loop.

In the run() method of Monitor, why is there a boolean named more, which is set to true, never changed, and only used to indicate that a loop runs forever? Why isn't it just a loop whose predicate is true, getting rid of the variable altogether?

What is to stop the value of _current from changing between the test at
Code: Select all
if (_highestPriority > _current && _current != NONE)

and the subsequent line
Code: Select all
 _behavior[_current].suppress();

? (Or, for that matter, between the first and second use of _current in the predicate in that first line?) After all, back in the original thread, you are in a loop where _current is alternately being set to _highestPriority or to NONE. If the Monitor thread executes until it is about to call
Code: Select all
_behavior[_current].suppress();

and then the original thread gets control and executes
Code: Select all
_current = NONE;

then yields to the Monitor thread, wouldn't you effectively execute
Code: Select all
_behavior[-1].suppress();

? I grant you, Java will treat you better here than would C++, or other languages that don't check every array indexing operation for legality, but still...
joseph.goldstone
Novice
 
Posts: 52
Joined: Wed Aug 27, 2008 10:27 pm
Location: San Francisco

Postby roger » Sat Nov 29, 2008 7:22 am

Hi Joseph,
Thanks for you analysis of the Arbitrator code. I am reworking it to fix the bug reported in this thread, and you have found a couple of additional problems. Here is a revised Montior thread loop that I think will avoid the problems you discovered. It seems work with some limited testing. Does it look OK to you?

Code: Select all
 public void run() {
            while (more) {         
                //FIND HIGHEST PRIORITY BEHAVIOR THAT WANTS CONTROL
                int wantsControl = NONE;
            for (int i = maxPriority; i >= 0; i--) {
                if (_behavior[i].takeControl()) {
                    wantsControl = i;
                    break;
                }
            }
             _highestPriority = wantsControl;
             int current = _current;
            if (wantsControl > current && current != NONE) {
                _behavior[current].suppress();
            }
                Thread.yield();
            }
        }


In this design, the contract for the action() method is : it exits only when the behavior action is complete or the suppress() method is called.
The contract for suppress() is: the action() method will exit quickly.

BTW,
The purpose of the first while loop in the start() method is that the loop immediate below would cause premature exit if _highestPriority has not been changed from the initial value of NONE.

Another question: would you prefer that the single parameter constructor call the two parameter version with the boolean parameter
true or false?
roger
Moderator
 
Posts: 368
Joined: Fri Jun 01, 2007 4:31 am
Location: Berkeley, CA

Arbitrator still dont`t work

Postby Tompolth » Tue Dec 02, 2008 11:29 pm

I'd been probing the arbitrator class with my robot, that before it works fine, and the arbitrator class still don´t work properly.

I was using the last revision included in the subversion repository (revision 2102, Mon Dec 1 20:34:41 2008) and I'd detected that suppress method only sometimes is executed. Althought the most times is not executed.

I was thinking if the problem would be because the threads of Arbitratror and Monitor is not synchronized, making that the shared variable _highestPriority change of value out of control.

Sorry for my English,
:oops:
Software Engineer
Tompolth
New User
 
Posts: 19
Joined: Sat Aug 30, 2008 6:29 pm

Postby lawrie » Wed Dec 03, 2008 9:20 am

Yes, I agree, it needs synchronization. We are currently looking into the best way to fix the Arbitrator. It should be fixed for the next release. You can always try out your own fixes as it is very easy to build your own version of classes.jar. When there is a working version in SVN, I will let you know.
lawrie
leJOS Team Member
 
Posts: 929
Joined: Mon Feb 05, 2007 1:27 pm

Postby Tompolth » Wed Dec 03, 2008 5:56 pm

lawrie wrote:Yes, I agree, it needs synchronization. We are currently looking into the best way to fix the Arbitrator. It should be fixed for the next release. You can always try out your own fixes as it is very easy to build your own version of classes.jar. When there is a working version in SVN, I will let you know.


I'd been reviewing the Arbitrator code. I believe that the problem is the next lines:

Code: Select all
private class Monitor extends Thread {

      boolean more = true;
      int maxPriority = _behavior.length - 1;

      public void run() {
         while (more) {
            // FIND HIGHEST PRIORITY BEHAVIOR THAT WANTS CONTROL
            int wantsControl = NONE;
            for (int i = maxPriority; i >= 0; i--) {
               if (_behavior[i].takeControl()) {
                  wantsControl = i;
                  break;
               }
            }

            // **** [b]HERE IS THE PROBLEM[/b]
            _highestPriority = wantsControl;
            int current = _current;
            // *************************

            if (wantsControl > current && current != NONE) {
               _behavior[current].suppress();
            }
            Thread.yield();
         }
      }
   }


and in these lines:

Code: Select all
public void start() {

      int lastActive = 1 + _behavior.length;
      monitor.start();

      while (_highestPriority == NONE) {
         Thread.yield();// wait for some behavior to take control
      }
      while (true) {
         if (_highestPriority != NONE) {
            if (_highestPriority != lastActive) {
               
               // ***** [b]HERE IS THE PROBLEM [/b]
               _current = _highestPriority;
               // **********************
               _behavior[_current].action();

               lastActive = _current;

               _current = NONE; // no active behavior at the moment

            }

         } else if (_returnWhenInactive) {
            monitor.more = false;// shut down monitor thread
            return;
         }
         Thread.yield();
      }

   }

In Start() method, the line

Code: Select all
 _current = _highestPriority;


take its value from _highestPriority variable that previously in run method in Monitor class take a value from the variable wantscontrol. So if the three commented lines are executed in this order:



Code: Select all
1-  _highestPriority = wantsControl;
            
                                                       2-   _current = _highestPriority;

 3-   int current = _current;


then the variables current & _highestPriority(and wantsControl) have the same value and never will be true the conditional

Code: Select all
  if (wantsControl > current && current != NONE)


I'll probe to repair this using two synchronized block around the commented lines but still haven´t achieved.
Software Engineer
Tompolth
New User
 
Posts: 19
Joined: Sat Aug 30, 2008 6:29 pm

Postby Tompolth » Thu Dec 04, 2008 5:01 pm

Hello again, guys:

I´d revised carefully the Arbitrator class code and I found the solution. It was more simple that I supposed. Only is necesary retire the next line in start() method:

Code: Select all
_current = NONE; // no active behavior at the moment


this line was responsible that the condicional

Code: Select all
if (wantsControl > current && current != NONE)


was ever evaluated to false, because current value was all times NONE.

I hope have been useful

Saludos desde España

:lol:
Software Engineer
Tompolth
New User
 
Posts: 19
Joined: Sat Aug 30, 2008 6:29 pm

Postby Shawn » Fri Dec 05, 2008 7:14 am

Tompolth wrote: Only is necesary retire the next line in start() method:

Code: Select all
_current = NONE; // no active behavior at the moment




Thanks! I commented out that line and it seems to work. This is with limited testing but so far so good!
User avatar
Shawn
Advanced Member
 
Posts: 723
Joined: Wed Sep 12, 2007 4:59 am
Location: Tokyo

Postby jrick » Tue Mar 10, 2009 1:43 am

I am running into this problem with my code, even when using false as the second argument when making my arbitrator object. I am currently testing with two behaviors. The first one runs, but then the arbitrator doesn't start the second (lower priority) behavior, even though the takeControl() method is set to always return true. How can I rebuild my classes.jar file with the new code to work around this problem?

Or even better, is there a new release sometime soon in the future? :D

By the way, I'm currently testing my robot on Windows, in case that makes any difference.
jrick
New User
 
Posts: 14
Joined: Wed May 07, 2008 9:07 pm

Next

Return to NXJ Software

Who is online

Users browsing this forum: Google [Bot] and 3 guests

more stuff