High-level activities
Opened this issue · 8 comments
(note: originally, I was using the word "scenarios", but I think "activities" is probably more accurate.)
Thinking through the design, here are the activities that see the bot doing:
- Detect defense type (necessary for autonomy mode only)
- Breach low bar
- Breach rough terrain
- Breach high wall
- Breach moat
- Breach ramparts
- Breach portcullis (stretch goal)
- Breach cheval (stretch goal)
- Turn clockwise
- Turn counterclockwise
- Collect boulder (with arm)
- Deposit boulder
In teleop mode, I guess we also need a general purpose move/drive activity. We could probably use this as the default activity, since nothing would happen until the driver moved the joystick. If we don't have this as the default activity, then we should probably have an "idle" activity.
Note that these activities are high-level concepts. We are not worrying about small things (like running motors, positioning arm at a given position, etc). Autonomy mode would string together a number of activities to do something. In teleop mode, we would have a button for each of the activities, where the driver is the one who's choosing what to do next.
So, what other activities would we need?
at the highest level those are all I can think of. I definitely think driver controls should be the default activity, not an idle one
possibly an activity for waiting at the secret passage drop point for a ball, which will extend the arm so the ball can go directly into the bucket, but then again that is almost simpler to do manually
Though it doesn't change the activities above, it occurs to me that we might be able to have a single "breach" button for teleop that uses the DetectDefenseType activity to figure out which BreachXXX activity to perform next. We can still have the individual activities available, but we probably wouldn't need them (unless the DetectDefenseType activity failed to figure out which defense the bot is seeing).
Also, for the High Wall, Rough Terrain, Mote, and Ramparts, we could probably simplify the list above by grouping them as a single BreachSimpleDefense activity. This would also make it easier for DetectDefenseType, as all of those defenses have the same detection pattern (i.e. nothing in front of the center range sensor).
So, we should be able to treat all of these activities as interchangeable objects. In each case, I see three actions we would want to perform on them:
- Initialize: called when an activity is selected as the current activity
- Update (called by the xxxPeriodic methods): this would be used to advance the internal state of each object
- Cancel: this would allow us to cancel an activity at any time (e.g. when switching to disabled mode)
So, start by creating a ABC (Abstract Base Class) called Activity (or rename Scenario) that has these abstract methods. We will be subclassing Activity for each of the activities above.
Next, add a new interface (that Robot will implement) called IRobotActivity (or something like that). This interface should define the following:
Activity GetDetectDefenseActivity()
Activity GetBreachLowBarActivity()
Activity GetDefaultActivity()
// etc.
void SetActivity(Activity activity);Note: that the GetDefaultActivity() method is going to return a different object depending on what mode we are in. For autonomy, the object will encapsulate our autonomy "intelligence". For teleop, this will be the ManualDriveActivity (or whatever we call it). For disabled, this will be a DisableActivity (a special activity that does nothing at all.)
Now, make sure that the Activity ABC constructor takes an IRobotActivity and stores it in a protected field. This will give the activities the ability to set what the next activity for the robot will be. For instance, in autonomousInit, we will call SetActivity(GetDefaultActivity()). Then, in autonomousPeriodic, we will call currentActivity.Update(). Because the first thing autonomy will do is detect the defense type, it's going to turn right around and call robotActivity.SetActivity(robotActivity.GetDetectDefenseActivity()) This will cause the DetectDefenseActivity object become the current activity, and subsequent calls to currentActivity.Update() will start the detection. That class will, in turn, call SetActivity() to whatever BreachXXXActivity it thinks is appropriate. Eventually, all activities end up calling robotActivity.SetActivity(robotActivity.GetDefaultActivity()). Depending on the mode, this will either return the robot to the appropriate "default" behavior.
There are a number of additional details we will need to fill in, but this should give us a good skeleton to work on. Incidentally, what you see above is a state machine, albeit a simple one.
I just want to point this out so we don't forget. We need to be sure to call all IRobotActivity methods on the singleton in Robot as we have no access to the instance made by FRC since it's declared directly in the main method....
Also thinking, should the ManualDriveActivity contain some sort of eventhandler to detect if an activity button was pressed or should it be separate? Since we don't want to be able to start another activity unless we are the default, being the manual drive I think it only detecting during then would be good
Actually, you do have access. Inside robotInit(), you can use 'this'. For instance:
@Override
public void robotInit()
{
_detectDefenseActivity = new DetectDefenseActivity(this);
//...
}
Typically, you could also add a default constructor for the class and do the same initialization there as well. My guess is that the reason robotInit() is usually used instead of the constructor is because there is other infrastructure that's initialized after the Robot object is created, but before robotInit() is called.
However, if you feel more comfortable creating a separate class, you can do that instead. That approach is just as valid. In that case, you probably don't need to bother with the interface; just define the class.
I also notice that Robot defines an INSTANCE static member, as well as makes the main components static. Just like the prior note, you can get rid of the INSTANCE and make the members non-static. Then, in the other classes (such as GameState), you pass the robot object to the constructor.
While the net effect is the same, there are some key reasons to avoid static instances. The most important one is that it is not obvious in the Robot class what other objects are referencing it. For instance, you cannot tell that the Auto class (which subclasses GameState) is itself referencing an instance of Robot.
The second reason is that INSTANCE is actually pointing to a different object than the one that FRC is creating. The only reason that this happens to work is that the rest of our members are static (shared by all instances). For instance, instead of accessing Robot.INSTANCE.arm, you could have simply done Robot.arm. In the case of a call like Robot.INSTANCE.isEnabled(), it just happens to work because isEnabled is using a singleton DriveStation object. But! if we add new members to Robot and forget to make them static, we will end up with two member values (the one set on the FRC Robot instance and the one set on the INSTANCE static instance).
Static scope has its place in object-oriented programming, but it should be used judiciously. I recommend you remove the static scope and the INSTANCE member. Then, for those classes that need a reference to Robot, pass the robot reference to those classes' constructors.
that makes a lot more sense