dirtbikerxz / BaseTalonFXSwerve

Other
145 stars 180 forks source link

After restarting the Robot We need to re align the wheels at the start #8

Closed varshil1999 closed 9 months ago

varshil1999 commented 1 year ago

After Switching off the robot and restarting it the wheels don't realign to the zero position we need to use a metal 1X1 pipe again to reset it and start the robot Please Do Help Us.

supermario6409 commented 1 year ago

@MrXpira Thank you for all the testing today, this was actually very helpful.

I think I've identified the issue to be a race condition between setting the invert and setting the sensor position, as in Phoenix 5, it's essentially the same issue as between setting the sensor position and reading it back. Adding a short (anywhere from 0.02-0.1 seconds) Timer.delay() directly before this line on the wait-for-cancoder-before-zero branch should fix the issue.

The waitForCanCoder() function should probably be left in, just because it is still theoretically possible if status periods are modified, and it is triggered in simulation, but on hardware it should just about always succeed on the first or second loop, and the timeout for the loop can probably be reduced.

Side note: this invert issue is not present in Phoenix Pro, as invert is simply another config in Pro.

We had just tested this suggestion on our MK4is and this solution does in fact work, we haven't tested with field oriented but the bot works as intended.

bhall-ctre commented 1 year ago

@MrXpira None of the other changes we made today (including the delay in the Swerve constructor) will be necessary, only what is currently on the wait-for-cancoder-before-zero branch in this repo and the additional Timer.delay() call in the SwerveModule.

dirtbikerxz commented 1 year ago

Like Ben said, thanks to @MrXpira's testing, we now believe the issue to be that motor inverts were not getting set fast enough before the offset position was being set to the angle motor. Specifically if you look at @MrXpira's testing you can see that from the 1st reset to the 2nd reset a second later, module 0 and module 3 negate their positions (5539 and 1328 to -5539 and -1317).

This also explains why only MK4i (and WCP Swerve X Flipped) users have been experiencing this issue, because only those modules require the angle motor to be inverted.

The solution to this is to add a delay between setting the motor invert and calling resetToAbsolute(). This fix has been implemented on the main branch, specifically here. The main branch with the fix has been tested on a real robot with MK4i's thanks to @Bankst's team, and is verified working as expected.

image

Theoretically this issue is resolved, but if anyone has any further problems please report them. Thanks again to everyone for the testing and feedback, would have been much harder to find the issue without it.

Dylan-Powers commented 1 year ago

I am finally coming back to this thread after seemingly reviving it. You can thank our lead design mentor for putting the resetToAbsolute() on a button binding, something I stubbornly did not do at his first request.

I have followed the conversation here and love many of the suggestions. Utilizing the waitForCanCoder() functionality looks pretty robust, especially if we also have the resetToAbsolute() on a button binding still (which we will be keeping since that is a good in-match troubleshooting tool). However, when I sat down today to clean up our Git mess and merge the new changes discussed here, I was confused as changes like this and this are not in BaseFalconSwerve/main. Instead, we seem to have gone with a simple delay on initialization. I am not necessarily opposed to that (it solves the mid-match brownout issue kind of), but I wonder why this decision was made when we had things like waitForCanCoder() working. Can someone shed some light on this? Perhaps @dirtbikerxz or @bhall-ctre?

dirtbikerxz commented 1 year ago

The waitForCancoder() method did not actually solve the issue since the issue had to do with setting inverts and not CANcoder data. The timer delay is required to give sufficient time for the inverts to be properly set before resetting the motors to absolute position. As you can see in @MrXpira's testing.

However, like Ben suggested the waitForCancoder() method should still be included in main, because theoretically that could be an issue (even though it is not the issue we were experiencing at this time). I just forgot to include it in main lol, will fix when I'm in front of a computer.

Dylan-Powers commented 1 year ago

@dirtbikerxz Thanks for the context! I also just realized that for those of us using the Thrifty Absolute Encoder, checking for errors from a CANCoder wouldn't exactly work.

Speaking of which, we have a few changes we are sitting on to make the Thrifty Absolute Encoders a drop-in replacement for the CANCoders. Right now we are still very much in the validation stage but eventually I can work on adjusting our code so that you guys could merge it with BaseFalconSwerve so that it can use the Thrifty Encoder as an option. However, if you would not merge that in any way, I would not bother with putting that all together, let me know what you think.

To be more specific, we essentially created a new class to mimic the base-level functionality of the CANCoder with just a simple analog absolute encoder like this one.

dirtbikerxz commented 1 year ago

Sure, the whole point of abstracting it into a separate getCanCoder() method was so that the angle encoder could be changed out in a single location, and everything else would just work. The only thing that matters is that the output of the getCanCoder() method is in degrees and CCW+. (I should really rename it from getCanCoder() to getAngleEncoder() or something). Same applies for the gyro.

bhall-ctre commented 1 year ago

However, when I sat down today to clean up our Git mess and merge the new changes discussed here, I was confused as changes like this and this are not in BaseFalconSwerve/main. Instead, we seem to have gone with a simple delay on initialization. I am not necessarily opposed to that (it solves the mid-match brownout issue kind of), but I wonder why this decision was made when we had things like waitForCanCoder() working. Can someone shed some light on this?

As @dirtbikerxz said, the issue was also caused by a race condition between setting inverts and setting the sensor position. Since the solution that ended up being implemented on main is a very long delay (1 second) followed by re-running the resetToAbsolute() calls, the waitForCanCoder() function is not really necessary, as the delay is excessive enough to cover both the invert issue and the bad CANcoder signal issue. If the implementation had instead done a short (20-100ms) delay immediately after setting the invert (such as here), then waitForCanCoder() would still be necessary.

Also keep in mind that waitForCanCoder() was only necessary because of the asynchronous nature of the CAN bus; non-CAN encoders do not have the same problem.

gsesny commented 1 year ago

@dirtbikerxz I am not sure if it related to what is happening here but it could be. We are running MK4i on Canivore with a pigeon2. The code works well except on module (module2) consistently does not get resetToAbsolute on initialization. (or that is what we think). Sometimes it gets seeded correctly but more often it does not. We get the sticky fault, "Remote Sensor is missing on Bus" when I run a selftest of that module 2 angle motor. We were however able to bind a button to resetToAbsolute and it fixes the issue every time. We are running the 1 second delay as prescribed in the swerve subsystem. Some other notes, I know we have no updated the Pigeon2 to the Current firmware and we may or may not have to look at the Canivore. The problem seems to becoming more frequent but it is only that module. The physical length of our can is about 30 feet. Anythoughts? We will do more testing on Monday, adding more resets, shortening Can and checking all firmware. We will also change out canCoder as the final step.

bhall-ctre commented 1 year ago

@gsesny A couple of things to test tomorrow:

gsesny commented 1 year ago

@bhall-ctre

Thanks for the help. I have a couple of things to report (I will admit I had a personal family illness with my kids and wasn't in the lab much the past two days).

-We are using the CANcoder as intended in the code. The CanCoder initializes the Talon FX as you stated and is how most teams are using it.

-We cleared all sticky faults. I have not seen the error "Remote Sensor is missing on Bus".
-We have shortened the length of the CAN Bus and separated the Swerve modules to be on the Canivore by themselves. ----The Canivore has been updated to latest firmware. -We are calling reset to absolute in robot.java during all disables and teleop and auto init. -We have not changed the order of initialization yet (that is my next step)

We have seen less issues with module but still there from time to time. Once we change the order of module boots and get data I will check back in.

We also noticed there is a can wire that has been almost cut all the way through. We will fix here shortly.

We may also change out the cancoder if necessary.

Gary-Delancy commented 1 year ago

I'm a little late to this game here, but we are having what i think is the same issue. Problem is, we are using MK2 hardware, and I'm not sure how/if to implement the same solution. I can't find any other promising ideas and we've wasted days messing around with this. Any suggestions on how to troubleshoot/solve this problem with MK2 hardware?

Wing57 commented 1 year ago

Like Ben said, thanks to @MrXpira's testing, we now believe the issue to be that motor inverts were not getting set fast enough before the offset position was being set to the angle motor. Specifically if you look at @MrXpira's testing you can see that from the 1st reset to the 2nd reset a second later, module 0 and module 3 negate their positions (5539 and 1328 to -5539 and -1317).

This also explains why only MK4i (and WCP Swerve X Flipped) users have been experiencing this issue, because only those modules require the angle motor to be inverted.

The solution to this is to add a delay between setting the motor invert and calling resetToAbsolute(). This fix has been implemented on the main branch, specifically here. The main branch with the fix has been tested on a real robot with MK4i's thanks to @Bankst's team, and is verified working as expected.

image

Theoretically this issue is resolved, but if anyone has any further problems please report them. Thanks again to everyone for the testing and feedback, would have been much harder to find the issue without it.

Ive been mulling over this issue by myself for a week to make sure that I wasn't making any stupid mistakes but I geniuenly cant find whats going wrong. I have the same issue demonstrated in the @gsesny 's video with the modules spinning wildly out of control. I've of course tried the solution you've provided but I've also tried the other things people suggested like moving resetToAbsolute() into Robot.java and still no luck. I recognize that this isnt a physical/hardware issue because when I copy this repository and modify the CAN ID's etc it works perfectly fine. To the best of my knowledge the code for the swerve between this and my own is identical (set aside variable names) so I'm lost as to why this is happening. I would geniunely appreciate any help/suggestions for this issue. This is my code: https://github.com/rambots/Viina/tree/Arm (Arm is the most up to date branch)

ericjanofski commented 1 year ago

I'm assisting a team with the same issues. We've put in the Timer.delay and it seemed to allow us to drive straight and strafe upon initial init, Turning didn't work. Then after disabling teleop, the pods spun a few times, then when we enabled teleop, the spun around a few more times. Very strange behavior!

https://github.com/steppj/7250-2023-Charged-Up-Code/blob/main/src/main/java/frc/robot

RedBread7576 commented 1 year ago

we also have encountered the same issue, with regular mk4 modules however. Assigning resettoabsolute() to a button and pressing it whenever the bug occurs is what we have done to overcome the issue

ericjanofski commented 1 year ago

Our solution with the mk4i was to set the angle motor invert to true. we also had to invert the drive motors. the config that was shipped with this code was incorrect.

ritchiedc commented 1 year ago

We have had a problem with a single wheel (module 3) coming up pointed wrong. It was intermittent and we made some changes (updating firmware, shortening can wires, etc) and the problem went away until our first competition. Our code has the 1 sec delay before the extra reset call which we left but we added a 1sec delay before the module creations as well giving more time for encoders and controllers to come up. All I can be sure of is that we did not have any more issues with wheel alignment for the remainder of the competition.

PS: almost forgot. I also put in a call to reset module 3 a second time in the reset function. Not knowing how the lower level stuff works I wanted to make sure there wasn't a problem with a buffer not being flushed or the like. When you're at competition you throw everything you can think of at the problem.

DoctorFogarty commented 1 year ago

I have an issue that I think is related to this where modules will lock 90 degrees off (like they ignored the offset) and we’re using the latest version of this code that has the 1 second delay in the swerve constructor. Something else has to be going on here.

I’m going to bind the ResetToAbsolute to a button tomorrow but so far this happens to us at every single meeting without fail so far on our MK4is.

https://github.com/MAikenMagic1102/Sheriff-ChargedUp-2023/blob/main/Sheriff-ChargedUp-2023/src/main/java/frc/robot/subsystems/Drive/Swerve.java

Every time the robot is enabled you can hear one or more modules spin around in place as well.

ritchiedc commented 1 year ago

@DoctorFogarty We had a similar problem where right/left became forward/backward, etc, but only for field oriented. For us it would happen mid match. Our driver was able to run the robot into the front wall and reset yaw a couple of times (bond to a button) and it would usually start working again. I suspect it may be pigeon2 related since it happened to all 4 modules and only in field oriented. If this sounds like your problem and you find a solution let me know and I'll do the same. I doubt it is related to this issue though.

hollisms commented 1 year ago

@ritchiedc do you use vision to update your odometry?

ritchiedc commented 1 year ago

@hollisms No we don't use vision but I see what your thinking. I would love to get vision working. Maybe next year (seems I say that every year). It would happen randomly. Now if it will only happen before the next competition so we can troubleshoot.

DoctorFogarty commented 1 year ago

The issue I’m describing only happens on startup I’ve never seen it happen where once we have the swerve working correctly it randomly locks up. We can run for extended periods of time without issue it’s always something that happens right after deploying new code that doesn’t effect the drive or on first robot startup to run. Our random tests in the shop don’t involve resetting our wheels to face the same direction at the start of a run, but I wonder if this problem ever happens when we do that.

github3658 commented 1 year ago

Can you also link me to your code. I looked on your GitHub profile and couldn't find it. Thanks!

We have used your code base , thank you!, for our swerve. When we use the left thumbstick all operations are as they should be. Using the right thumbstick for rotation does not work at all. Instead of the modules going to a 45 degree allowing the chassis to turn in a O they are pointed to the center of the chassis forming an X.

github3658 commented 1 year ago

Our solution with the mk4i was to set the angle motor invert to true. we also had to invert the drive motors. the config that was shipped with this code was incorrect.

How to you set the invert if you are using Mk3 modules instead of the Mk4?