Extending the Talon Class and using with RobotDrive

[AlexanderTheOK]

We have a class that is an extension of the Talon class. It adds some functions such as current limiting.

As it extends the Talon class, which implements SpeedController, it would be reasoned that we should be able to pass our "Custom Talons" to other classes such as the RobotDrive class.

Unfortunately, it seems that when passing out extension of Talon, robotDrive doesn't seem to call the overridden set function. For the sake of making our code easier to read we've put only the pertinent parts into a separate project and reproduced the bug there. Here is that code from an iterative robot project.

Robot.java:

public class Robot extends IterativeRobot {
    final String defaultAuto = "Default";
    final String customAuto = "My Auto";
    String autoSelected;
    SendableChooser chooser;
    
    TestTalon A = new TestTalon(2),
    		B = new TestTalon(3),
    		C = new TestTalon(0),
    		D = new TestTalon(1);
    
    RobotDrive drive = new RobotDrive(A,B,C,D);
    
    Joystick xbox = new Joystick(0);
	
    /**
     * This function is run when the robot is first started up and should be
     * used for any initialization code.
     */
    public void robotInit() {
        chooser = new SendableChooser();
        chooser.addDefault("Default Auto", defaultAuto);
        chooser.addObject("My Auto", customAuto);
        SmartDashboard.putData("Auto choices", chooser);
    }
    
	/**
	 * This autonomous (along with the chooser code above) shows how to select between different autonomous modes
	 * using the dashboard. The sendable chooser code works with the Java SmartDashboard. If you prefer the LabVIEW
	 * Dashboard, remove all of the chooser code and uncomment the getString line to get the auto name from the text box
	 * below the Gyro
	 *
	 * You can add additional auto modes by adding additional comparisons to the switch structure below with additional strings.
	 * If using the SendableChooser make sure to add them to the chooser code above as well.
	 */
    public void autonomousInit() {
    	autoSelected = (String) chooser.getSelected();
//		autoSelected = SmartDashboard.getString("Auto Selector", defaultAuto);
		System.out.println("Auto selected: " + autoSelected);
    }

    /**
     * This function is called periodically during autonomous
     */
    public void autonomousPeriodic() {
    	switch(autoSelected) {
    	case customAuto:
        //Put custom auto code here   
            break;
    	case defaultAuto:
    	default:
    	//Put default auto code here
            break;
    	}
    }

    /**
     * This function is called periodically during operator control
     */
    public void teleopPeriodic()
    {
     double move = xbox.getRawAxis(1);
     double rotate = xbox.getRawAxis(4);
           
     drive.arcadeDrive(move, rotate);
     
    }
    
    /**
     * This function is called periodically during test mode
     */
    public void testPeriodic() {
    
    }
    
}

TestTalon.java:

public class TestTalon extends Talon
{
	
	public TestTalon(int channel) {
		super(channel);
		// TODO Auto-generated constructor stub
	}

	@Override
	public void set(double value)
	{
		System.out.println("TestTalon running");
		super.set(value);
		
	}
}

The idea here is that we should be seeing "TestTalon running" being printed to the riolog four times every loop of the function. Nothing is sent to the riolog, moreover, the robot continues to drive fine, indicating that it's running the Talon code but not the code from our extension.

Is there something that we're missing about how java handles inheritance or is there something strange going on with the WPILibs?

[rrossbach]

Originally Posted by AlexanderTheOK

[SNIP]...robotDrive doesn't seem to call the overridden set function....[SNIP]

Precisely. Looking at the wpilibj source, RobotDrive.java never calls set(double), it only calls set(double, int) which you haven't overridden.

That latter method has a second parameter to handle the CANJaguar "sync group" - when not using a sync group it's just set to 0; and since you're not extending CANJaguar you can safely ignore it.

So, just override the set(double, int) method, rather than set(double), and it should work.

Hope that helps!

[AlexanderTheOK]

wow... We completely would not have noticed that, especially since that set method is considered deprecated in the Talon class.

So thanks an actual metric tonne, because it works now.

[tcjinaz]

Originally Posted by AlexanderTheOK

wow... We completely would not have noticed that, especially since that set method is considered deprecated in the Talon class.

So thanks an actual metric tonne, because it works now.

To better understand the tools I'm learning this year, might not Eclipse flag this sort of disparity? At least flagging the reference/overload/whatever of the deprecated method? It has certainly not been shy about letting me know when I lay in a deprecated anything.

Just an uneducated question from a Java/Eclipse rookie.
Tim

[AlexanderTheOK]

Originally Posted by tcjinaz

To better understand the tools I'm learning this year, might not Eclipse flag this sort of disparity? At least flagging the reference/overload/whatever of the deprecated method? It has certainly not been shy about letting me know when I lay in a deprecated anything.

Just an uneducated question from a Java/Eclipse rookie.
Tim

The idea is that every motor controller implements the speedcontroller interface. This means they all have the same functions but the functions work differently between say a talon and a jaguar.

This allows the RobotDrive class to say "i want a speedcontroller but i dont care which one."

very useful but it needs to be used carefully. This is where things get tricky for me, so if im wrong someone will need to correct me.

The issue in this case is that jaguars have "syncgroups" and regular talons dont. However the speed controller interface defines the set method that uses syncgroups, so every class extending it now has to at least define it, whether or not it is actually something it uses or not.

I mean, imagine if you passed a talon into the drive code, and it tried to use that set method, and it just wasnt there.

So instead of rewriting everything that used motor controllers to make it choose whether or not to use sync groups based on what kind of controller it was, they left the original "set(speed,syncgroup)" method in for classes that didnt use syncgroups (such as the talon), and had it do the same thing that set(speed) did and ignore the syncgroup variable. Call it deprecated and be done with it.

Drive code can still give the talons a syncgroup and you can just ignore the deprecated method when writing your own code. nothing is technically wrong.

I feel that a better way to write this however would be to have the deprecated method simply call the new one and throw out the syncgroup, instead of copy pasting the code. This means that no matter what happens, both functions have to do the same thing for classes without syncgroups so extending either function works, otherwise you get confused teams.