CrossTheRoadElec / Phoenix5-Examples

HERO C#, FRC C++/Java, future platforms.
116 stars 169 forks source link

Problem with simulated CANCoder #61

Open Crossle86 opened 2 years ago

Crossle86 commented 2 years ago

I have wrapper classes for SRX & FX encoders. Cloned the FX encoder class to create to wrapper for CANCoder. The code that drives the sim updates: ` public void setSimValues(double position, double velocity) { simCollection.setRawPosition(metersToTicks(position));

    simCollection.setVelocity(velocityToTicks(velocity));
}

'

Reset is done with: ` public void reset() { ErrorCode errorCode;

    errorCode = encoder.setPosition(0);

    if (errorCode != ErrorCode.OK) 
        Util.consoleLog("encoder reset failed (%d) %s", errorCode.value, errorCode.name());
}

` Running under sim, the absolute position works fine but the encoder count returned by get() is way out of whack. Start sim do no driving, just call for reset of encoder, the next (and all subsequent) get() returns a huge negative number. Then driving some does not change that value returned by get(). Note that I get this error on the reset() call: encoder reset failed (-3) CAN_MSG_NOT_FOUND

CoryNessCTR commented 2 years ago

What you're seeing is not expected behavior. I'm also unable to reproduce what you're seeing.

My process:

  1. Take fresh CANcoder Java example
  2. Add following code to Robot.java around line 165

    // Don't forget to import CANCoderSimCollection at the top
    /* New code */
    CANCoderSimCollection simCollection = _CANCoder.getSimCollection();
    @Override
    public void simulationPeriodic() {
    simCollection.addPosition(1);
    simCollection.setVelocity(20);
    }
    
    /* Existing code */
    @Override
    public void teleopInit() {
    _CANCoder.setPosition(0); // New line
    }
  3. Simulate and occasionally teleop-enable to reset CANcoder, position and absolute position both look healthy and never see errors

Could you provide a process similar to what I did that reproduces the issue?

FYI, triple backtick will allow you to paste multiple lines of code, and you can specify language on the first line, like so:

```java
java code
Crossle86 commented 2 years ago

So I did more testing and found two things: 1) I was resetting the count in my cancoder wrapper class constructor. This where the -3 error happened. Reset later worked no error. 2) Adding a reset outside constructor fixed the problem with strange counts returned by getPosition().

The cancoder sim is working now. Perhaps the failed reset call was causing some corruption and why doing reset in constructor is a problem I sure don't know, but looking good now.

I worked with you earlier on encoder (SRX) reset in sim and problems related to timing. I adjusted my timings and things seemed fine then, but testing today on the cancode issue revealed I am still seeing pretty strange issues on resetting an SRX, waiting longer than the frame speed, seeing zero returned and then not long later the old pre-reset counts reappear. I will file a new issue after I look at this a bit more.

Crossle86 commented 2 years ago

So I think the error on reset in constructor is the robot is not yet enabled and so signals to hardware are not transmitted by the Rio causing the cancoder library to return the error. Thoughts?

CoryNessCTR commented 2 years ago

I looked further into this, what you're seeing is close to what you think is happening.

Phoenix need some sensor configuration information from CANCoder when it performs the setPosition operation. This is normally not an issue in hardware since CANCoder always sends out the CAN frames, so the RIO will receive frames during its boot process. However when you construct a CANCoder and use it immediately afterward while simulating, there hasn't been enough time for Phoenix to receive the CAN frames. If you wait 250ms it should be sufficient for Phoenix to receive the necessary CAN frames. Alternatively, if you're setting position to 0 the operation will work without issue and you can ignore the error message.

Crossle86 commented 2 years ago

So when I set the position to zero in the class constructor and ignored the error, the position did not reset to zero, getposition returned a nonsense value that did not change during the test run. If I waited, like you suggest, then set position to zero, that worked: getposition returned zero and incremented as expected after that.

While what I observed and what you described don't seem to quite match up, I am good with calling this one solved.