Team2168 / 2015_Main_Robot

Source code for the 2015 season
2 stars 0 forks source link

Modify lift subsystem to zero outputs to motors if it has reached end of travel #83

Closed jcorcoran closed 9 years ago

jcorcoran commented 9 years ago

Right now the drive method must be called repeatedly for the hard stop statuses to be used to stop the drive, this leads to problems (lift continues to command through hard stop) if a command ends having last told the lift to move at a non zero value.

Ideally the subsystem would monitor these states itself and not be dependent on the commands/scheduler.

NotInControl commented 9 years ago

Do you have a fix in mind?

This is only possible in auto because the joysticks are not getting values sent by the ds periodically.

I think a quick reliable fix would be to modify the drive with joystick commands, and in the execute block add

If robot is in auto drive 0.

That should solve this issue without having to add threads for each subsystem and possibly screw up the scheduler

NotInControl commented 9 years ago

Actually... what I suggest may not work. If I remember correctly, when running a commandgroup the active command on each subsystem is the commandgroup, not the individual command requiring the subsystem.

So if the commandgroup is still running even though you killed the command on the lift subsystem inside the commandgroup, the lifts default command doesnt run, because the active commandgroup is still running... if it did we wouldnt have the problem.

Ignore my suggestion above... we need to think through this more

NotInControl commented 9 years ago

Couldnt we just enable motor safety on the motor controllers by calling the setSafetyEnabled method of each motor controller in the subsystem?

Wouldnt that solve this?

jcorcoran commented 9 years ago

That's an option, I wanted to look at the implantation before using it though, I don't know what the timeout period is in their code.

NotInControl commented 9 years ago

The default expiration time is 0.0. Only the controllers extend safePWM (i.e. Talon, Victor... etc) so for the drivetrain we would either need to change the controllers from the SpeedController type to Talons, or typecast to Talons in order to access its SafePWM inherited methods.

In addition to enabling safePWM using the setter, you can also set the timeout using the setExpiration(double) method. The safe Robot Drive classes, use 0.1 as their timeout, which I think is in seconds, so we could start there.

So the fix should be as simple as calling the below on every motor object:

public static final double kDefaultExpirationTime = 0.1;
leftMotor1.setExpiration(kDefaultExpirationTime);
leftMotor1.setSafetyEnabled(true);
jcorcoran commented 9 years ago

Yea, enabling motor safety and setting the timeout period from within the subsystem is probably the easiest way to go.

I think it would be fine to instantiate the controllers of type Talon or Victor. As we have commonality of controllers between the two robots. If that changes later we can deal with it then.

I wouldn't enable motor safety anywhere but the DT and Lift subsystems.

NotInControl commented 9 years ago

Ok, after the PID branch becomes master, Ill have @Dustin-Garner fix this.

jcorcoran commented 9 years ago

I'll try to do the merge tonight. Not really looking forward to it lol.

NotInControl commented 9 years ago

In the end, the PID branch is master, so its not really a merge more than it is a replace. Actually, I would prefer you just do a replace than a merge, because I am not sure how some of the package renames are going to come through. If I understand how the merge process works, because those changes came after master, even if you did merge, it should delete the old package names and replace them with the new ones correct, so a merge should be fine, right? All the commits are ahead of master, so you shouldn't have any conflicts correct?

jcorcoran commented 9 years ago

Yea that's fine, but some issues were resolved in master that I don't believe made it into paid branch. I'll try to cherry pick commits.

NotInControl commented 9 years ago

I dont know how thats possible, wouldnt that mean that the PID branch would have some commits behind master? But it doesnt. I am pretty sure the last time I merged master nothing was changed.

But if what ur saying is true, then I would still force PID to be master, overwriting any deltas and if we need any fixes that were in master we write them and add them again.

I am very familiar with the code in the PID branch, and that is the code I want to call master so that I know any changes we make moving forward are deterministic based on the performance and behavior we had at PVDE.

If master does have new code, that was not in the PID branch, we were never running it at comp, so I would not merge that into the PID branch, or whatever we are now calling master.

I think the easiest way to move fwd would be to replace master with the PID branch and delete all stale branches... this way we have a reset on the code and can move fwd in a deterministic way. If that means we have to do a little rework then so be it

Sent on a Sprint Samsung Galaxy S® 5

-------- Original message -------- From: James notifications@github.com Date:03/11/2015 4:57 PM (GMT-05:00) To: Team2168/2015_Main_Robot 2015_Main_Robot@noreply.github.com Cc: Kevin kevinharrilal@msn.com Subject: Re: [2015_Main_Robot] Modify lift subsystem to zero outputs to motors if it has reached end of travel (#83)

Yea that's fine, but some issues were resolved in master that I don't believe made it into paid branch. I'll try to cherry pick commits.


Reply to this email directly or view it on GitHub: https://github.com/Team2168/2015_Main_Robot/issues/83#issuecomment-78369897