JAVA Command Group Question

[buckskinner1776]

We have been using JAVA for a couple of years now and each year have had issues with the command groups. Previously, for the most part, sequential commands have worked fine but parallel commands have had hit and miss success. We've been through the manuals repeatedly without any success in clearing up the issues.

This year it's been a little better but we just ran into an issue that makes me wonder if it isn't what we were seeing before.

We have a number of commands added sequentially in a command group. These commands for the most part run fine. The problem we get to is when the output of one command is used in a subsequent command. We might initialize the robot with a robot.drivedistance variable initialized to 10". The first command in the command group may set the robot.drivedistance to 100". Then a command further down in the command group is passed this distance such as driveRobotForDistance(robot.drivedistance, fullspeed). This command seems to execute with the initial 10" instead of the 100".

Can anyone explain why? We can hardcode the 100" into driveRobotForDistance(100,fullsspeed) and it works just fine.

[GeeTwo]

It's impossible to be sure without seeing the code, but it sounds like problems in variable scope, that is, a confusion between global and local variables which have the same name.

[Pault]

This is a really annoying aspect of CommandGroup. Basically, all the code that you write for a CommandGroup is in the constructor. This means that the code itself is going to run instantaneously, probably when you first turn on the robot. What addSequential() and addParallel() do is pretty much just add the commands you requested into a queue. Those commands are then iterated through later when the CommandGroup is actually started.

What this means is that if you have a 3 line command group, like such:

Robot.drivedistance = 10;
addSequential(new Command1());
addSequestial(new Command2(Robot.drivedistance));

with Command1 containing the following line in its initialize method:

Robot.drivedistance = 100;

When the robot is turned on, the 3 lines of code in your CommandGroup will be executed. This means that drivedistance will be set to 10, then a new Command1() will be created and added to the queue, then a new Command2 will be created with the current value of Robot.drivedistance (which is 10) as the parameter, and added to the queue. When this command is actually run, Command1 will change Robot.drivedistance to be 100, but that doesn't matter because Command2 has already been told the distance it is supposed to drive.

There are a few different ways to fix this, although I don't consider any of them pretty. The simplest way is probably just to create a new command that will drive the distance specified by Robot.drivedistance, and have it be separate from your normal driveRobotToDistance command. In this new command you should check what the value of Robot.drivedistance is in the initialize method, not the constructor.

[buckskinner1776]

Yes. I don't see it as being a scope issue. I just caught yesterday that the commands are actually in the command group constructor. Had never noticed that before. Explains why we couldn't run if structures in it.

We came to the same conclusion. Use the initialize method to check the global. This could very well explain the behavior that we'd seen the previous years as well. It was kind of fuzzy and only seen at certain times.

We were coming to the conclusion that all of the commands in the command group were being instantiated early. Our previous train of thought is that they would be instantiated when we instantiated the command group. A few system.outs for the poor man's trace seemed to be showing us otherwise.

this link: http://www.chiefdelphi.com/forums/sh...ommand+gro up has some interesting concepts to get around it as well. But it was a little past my experience and I don't want to tackle that at t-3 days.

[notmattlythgoe]

Originally Posted by GeeTwo

It's impossible to be sure without seeing the code, but it sounds like problems in variable scope, that is, a confusion between global and local variables which have the same name.

It is obvious that you have no understanding of how the CommandGroups work. Please stick to answering questions about topics you know and avoid just trying to answer every single question that is posed.

[euhlmann]

Originally Posted by notmattlythgoe

-snip-

Let's not be harsh here. @GeeTwo is correct that we'd need the code to fully understand what's going on.

---

For complicated command logic, the CommandGroup often isn't enough. What we did by NECMP this year was write our own CommandGroup-like class to run our automatic shooting logic (it was a lot prettier than the mess of CommandGroups in CommandGroups we had before).

When you have full control over how sub-commands are run, it's easy to relay the values you need. And, in terms of good programming practice, it's the best solution.

[MrRoboSteve]

Originally Posted by buckskinner1776

This command seems to execute with the initial 10" instead of the 100".

Lacking source code, both Gus and Pault's theories could easily be true. They're both theories about scoping and order of evaluation.

As an aside, I think Pault's criticism of the CommandGroup is more about the subtleties of Java's pass-by-value semantics. A statement like:

addSequestial(new Command2(Robot.drivedistance));

means "create a command with the current value of Robot.drivedistance". At the point the JVM news up a Command2, it takes whatever the value is of Robot.drivedistance, and passes it in as a parameter.

Here is an easy way to use CommandGroups to achieve what Pault is desiring, without a lot of extra code.

Suppose you have two Commands, CalculateDistanceToShootingPosition and DriveForward. You want to execute them in sequence in a CommandGroup. You could say something like this in the CommandGroup's constructor:

DriveForward df = new DriveForward();
CalculateDistanceToShootingPosition calc = 
  new CalculateDistanceToShootingPosition(df);
addSequential(calc);
addSequential(df);

Now, suppose that DriveForward has a method SetDistanceToDriveInInches(). In CalculateDistanceToShootingPosition.Execute(), you'd do the calculation and then call SetDistanceToDriveInInches(). Later, when DriveForward runs, it has the correct value to drive to. This uses the correct value, and eliminates a global variable, making DriveForward more flexible and easier to reason about.

[Pault]

Originally Posted by MrRoboSteve

As an aside, I think Pault's criticism of the CommandGroup is more about the subtleties of Java's pass-by-value semantics. A statement like:

addSequestial(new Command2(Robot.drivedistance));

means "create a command with the current value of Robot.drivedistance". At the point the JVM news up a Command2, it takes whatever the value is of Robot.drivedistance, and passes it in as a parameter.

Here is an easy way to use CommandGroups to achieve what Pault is desiring, without a lot of extra code.

Suppose you have two Commands, CalculateDistanceToShootingPosition and DriveForward. You want to execute them in sequence in a CommandGroup. You could say something like this in the CommandGroup's constructor:

DriveForward df = new DriveForward();
CalculateDistanceToShootingPosition calc = 
  new CalculateDistanceToShootingPosition(df);
addSequential(calc);
addSequential(df);

Now, suppose that DriveForward has a method SetDistanceToDriveInInches(). In CalculateDistanceToShootingPosition.Execute(), you'd do the calculation and then call SetDistanceToDriveInInches(). Later, when DriveForward runs, it has the correct value to drive to. This uses the correct value, and eliminates a global variable, making DriveForward more flexible and easier to reason about.

That is an interesting way of doing it. But I still stand by my annoyance with CommandGroups. They are extremely limited in what they can do, and that is a result of the way they are designed. Sure there are some clever workarounds like that which can make it do things it normally cant, but the fact that you need to be that clever just to make any sort of decision live is a huge flaw. There are plenty of other ways they could have been designed to avoid these problems. Over the summer I decided to code my own version, and my way of working around these problems was simply to put the execution of the CommandGroup on a separate thread, that way the method could be run live and actually iterate through the code in real time instead of just throwing a bunch of commands in a queue.

Right now the code is has a couple of bugs in certain edge cases, but I hope to release it publicly soon.

[Ether]

Originally Posted by Pault

Over the summer I decided to code my own version ... I hope to release it publicly soon.

What's your opinion of CCRE?

[Pault]

Originally Posted by Ether

What's your opinion of CCRE?

I've never taken the time to look at it in depth, but overall it seems like an extremely capable framework. I am afraid to use it however, because it is not nearly as widely used (and therefore widely tested) as the standard WPILIB. Also, I feel that because it is managed by a team and not people directly affiliated with FIRST, there is much less of a guarantee that it will be kept up to date, and if it were to ever become obsolete it would be a fairly painful transition back to normal.

246's programming department has grown immensely over the past 2 years, and looks to be continuing on an upward trend. I would rather take a few years to build up a coding foundation that is tailored exactly to what we want, we can trust, know the limitations of, and know how to fix if something goes wrong.

[Ben Wolsieffer]

Originally Posted by Pault

That is an interesting way of doing it. But I still stand by my annoyance with CommandGroups. They are extremely limited in what they can do, and that is a result of the way they are designed. Sure there are some clever workarounds like that which can make it do things it normally cant, but the fact that you need to be that clever just to make any sort of decision live is a huge flaw. There are plenty of other ways they could have been designed to avoid these problems. Over the summer I decided to code my own version, and my way of working around these problems was simply to put the execution of the CommandGroup on a separate thread, that way the method could be run live and actually iterate through the code in real time instead of just throwing a bunch of commands in a queue.

Right now the code is has a couple of bugs in certain edge cases, but I hope to release it publicly soon.

This is kind of getting off topic, but my solution to this problem is to use Double/Boolean/WhateverSuppliers for the parameters, rather than passing the actual values. Since we are using Java 8, I just pass a lambda or method reference into the constructor. This adds a little complexity to the command code, but gains a lot of flexibility. For example, it makes it possible to update a parameter while the command is running.

[MrRoboSteve]

Originally Posted by Pault

Over the summer I decided to code my own version, and my way of working around these problems was simply to put the execution of the CommandGroup on a separate thread, that way the method could be run live and actually iterate through the code in real time instead of just throwing a bunch of commands in a queue.

Hard to see how multiple threads will improve things for teams that are tripped up by Java's call by value semantics.

[Pault]

Originally Posted by MrRoboSteve

Hard to see how multiple threads will improve things for teams that are tripped up by Java's call by value semantics.

Again, I don't see this as a problem with the way Java works, I see this as a problem with the way CommandGroups use Java. There are plenty of workarounds to make CommandGroup work if you understand why it is so limited, but every suggestion that I have seen is code that I consider fairly ugly and overly complicated. I would much rather use a framework that allows for real time decision making. That means more than just being able to pass variables into Commands, it also includes having if statements and while loops, and the capacity to be more specific about when a Command should start other than just addSequential and addParallel. The 2 best ways I can think of doing this are a state machine and a separate thread. State machines are hard to create a CommandGroup like framework for, and overall take a significantly longer time to code. But it is possible to have something very similar to CommandGroup on a separate thread, with 1 liners that will run a Command once certain parameters are met.

[MrRoboSteve]

When you decide to publish your code, please loop me in. I'm interested in seeing what you propose.