Chief Delphi - Call to PIDController::GetError() and ::OnTarget() locks program.

Chief Delphi (http://www.chiefdelphi.com/forums/index.php)

- C/C++ (http://www.chiefdelphi.com/forums/forumdisplay.php?f=183)

- - Call to PIDController::GetError() and ::OnTarget() locks program. (http://www.chiefdelphi.com/forums/showthread.php?t=111990)

KGU	24-01-2013 20:15

Call to PIDController::GetError() and ::OnTarget() locks program.

We have been working with the PID controller and have a simple set up with a motor and potentiometer. The motor tries to control to set point as it should, but only if the we are not using the GetError or OnTarget methods. If either of these methods are called, the program locks up and does not continue on to the rest of the code. Here is a snippet of our code (we initialize everything in a separate header not shown):

Code:

CRobotMain::CRobotMain() : SimpleRobot()

{

        // Create our object pointers.

        m_pPot                = new AnalogChannel(1, 2);

        m_pMotor        = new Talon(1, 8);

        m_pPID                = new PIDController(0.1, 0.001, 0.0, m_pPot, m_pMotor);

}



void CRobotMain::OperatorControl()

{

        float        fError;

        bool        bOnTarget;

        

        m_pPID->SetOutputRange(-0.25, 0.25);

        m_pPID->SetInputRange(0, 1023);

        m_pPID->SetAbsoluteTolerance(50);

        m_pPID->SetContinuous(true);

        m_pPID->Enable();

        

        // Continuously run loop while in teleop mode.

        while (IsOperatorControl())

        {

                m_pPID->SetSetpoint(250);



///                fError = m_pPID->GetError();

///                bOnTarget = m_pPID->OnTarget();

                

                Wait(0.050);

        }

}

If either of the "m_pPID->GetError();" or "bOnTarget = m_pPID->OnTarget();" lines are uncommented, the code no longer functions.

Joe Ross

24-01-2013 20:38

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

See if the following bug report seems like it could cause your issue: http://firstforge.wpi.edu/sf/go/artf1595

jwakeman

25-01-2013 00:06

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

I ran into this tonight as well. Last year the implementation of PIDController:GetError() just set error = m_error inside the critical section. Anyone know the reason for now trying to set error = GetSetpoint() - m_pidInput->PIDGet()? Seemed like just returning m_error was good enough..

jwakeman

04-02-2013 09:12

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Quote:

Originally Posted by jwakeman (Post 1221529)

After this weekend I think I understand now why they attempted to update the GetError() method this year. With last year's implementation the following code results in a race condition. It will not work if the OnTarget() method ends up running before the PID gets a chance to run its calculation loop at least once.

Code:

myPID->SetSetpoint();

while(!myPID->OnTarget()){}

Jefferson

04-02-2013 18:35

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Replace your GetError function with this. It should work.

/**
* Retruns the current difference of the input from the setpoint
* @return the current error
*/
float PIDController::GetError() {
float error;
PIDSource *pidInput;
CRITICAL_REGION(m_semaphore)
{
pidInput = m_pidInput;
error = GetSetpoint() - pidInput->PIDGet();
}END_REGION;
return error;
}

JDNovak

05-02-2013 02:46

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Scratch that last post. I thought this fixed the issues I was having during the Beta season, but it's blowing up for me now. If anybody has a work-around, I'd love to try it out.

Jefferson

05-02-2013 02:48

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

And ^ that's what I get for not checking who is logged in to CD on the programming computer!

Sorry John. I'm going to bed now.

jwakeman

05-02-2013 09:50

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

I don't have the code in front of me right now but i think i ended up using this:

Code:

float PIDController::GetError() 

{

   float error;

   CRITICAL_REGION(m_semaphore)

   {

      error = m_setpoint - pidInput->PIDGet();

   }END_REGION;

}

return error;

Also in the OnTarget() method I do the same calculation to get the error instead of calling GetError().

Even with this updated implementation of GetError() I also learned last night not to call GetError() within PIDWrite().

BradAMiller

06-02-2013 16:25

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Sorry about the bug. There will be a fix posted shortly as soon as we finish fixing another issue with SmartDashboard.

Basically the problem is that the critical region uses a binary semaphore which blocks on multiple takes from the same thread. The code has a bunch of CRITICAL_REGIONS nested so it's hanging.

The fix was to replace the semaphore with a Mutual Exclusion semaphore which will block on multiple tasks trying to access the code, but not when the same task tries.

Brad

jwakeman

06-02-2013 17:15

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Quote:

Originally Posted by BradAMiller (Post 1228938)

Sorry about the bug.

No need to apologize! Thanks for your hard work and support! I just felt the need to say that because I have seen a few posts on CD where folks are incensed because they have found an issue with WPILib. I try to remind myself that they are probably just young kids or don't understand that WPILib is put out by a few hard working people (volunteers?) and not some large corporation. When Microsoft Word crashes and takes all my work with it I get mad but when I find an issue with WPILib I try to report it and offer a solution if possible.

Jefferson

06-02-2013 18:00

Re: Call to PIDController::GetError() and ::OnTarget() locks program.

Quote:

Originally Posted by jwakeman (Post 1228969)

No need to apologize!

Seconded! We appreciate your hard work, Brad.

All times are GMT -5. The time now is 12:55.