Skip to content

Update from Phoenix 5 to Phoenix 6 - #62

Closed
ProfessorAtomicManiac wants to merge 14 commits into
masterfrom
update-phoenix
Closed

Update from Phoenix 5 to Phoenix 6#62
ProfessorAtomicManiac wants to merge 14 commits into
masterfrom
update-phoenix

Conversation

@ProfessorAtomicManiac

Copy link
Copy Markdown
Contributor

No description provided.

@ProfessorAtomicManiac ProfessorAtomicManiac added the Breaking Change This PR will require an increment to the lib199 major version number label Jul 24, 2023
@ProfessorAtomicManiac

ProfessorAtomicManiac commented Jul 24, 2023

Copy link
Copy Markdown
Contributor Author

Originally we made the decision to update from Phoenix 5 to 6 so that when the CANcoders on the drivetrain are disconnected, the robot will switch to relative encoders. (Not yet tested, will update if updating to Phoenix 6 fixes the issue).

However, Phoenix 6 has a ton of changes, making this PR a very dangerous change. From what I gathered (Please double check that I got everything here correct):
-TalonSRX and VictorSPX are no longer supported. The library only supports TalonFX.
-All motor controllers and encoders support simulation and implement WPILIB's MotorController interface. Therefore all mock classes are no longer necessary and should be removed. This also means ErrorCodeAnswer.java will not be used, so maybe consider removing it.
-Nominal output has been removed. In general please double check I did the config for TalonFXs correctedly, since I couldn't find exact matches for some of the options. (Ex: DutyCycle in Phoenix 6 ?= Continuous in Phoenix 5). I also couldn't find where to put in timeouts for some of the functions.
-CANcoders only return output from [0, 1] or [-0.5, 0.5] in rotations.

@CoolSpy3

Copy link
Copy Markdown
Member

Wow! I did not realize so much had changed. I think we might need to reevaluate what we need to update. In order:

  • Can confirm. This might be because Phoenix 6 is actually just public Phoenix Pro and is only made to target CANCoders and FX series products. We might have to compromise and download the combined Phoenix 5/Phoenix 6 version to maintain support for these controllers. If we do axe them, it MUST be a leadership discussion because I know we have a lot of Talon SRXs lying around, and (at the moment) lib199 does not support using Spark Maxes in brushed mode.
  • See above for why we might need to keep this, however, I wouldn't put it past CTRE to have fixed any simulation problems by now. We should look into whether this code is still necessary. Also, can you double check that all the necessary endpoints are exposed in their simulation implementation. Also check if they have a gearing function and how that plays with simulation. If they don't have one, we need to implement it. (This is similar to Unnecessary Simulation Check #44)
  • If we aren't using Talon FXs, I wouldn't implement code for them.
  • This shouldn't be a problem AFAIK, the only place we use them is one or two lines in SwerveModule

@ProfessorAtomicManiac

Copy link
Copy Markdown
Contributor Author

2024 update to Phoenix allows us to have both Phoenix 5 and Phoenix 6 as dependencies. I would opt for including them both so we can keep our old features while also giving us the ability to use new motors such as the Krakens. The CANCoder class is deprecated and it is recommended to use the v6 version. Phoenix 5 will stay so we don't have to remove the CANCoder classes, but I'd say we should do it now that we can safely install Phoenix 6.

@brettle

brettle commented May 14, 2024

Copy link
Copy Markdown
Member

@ProfessorAtomicManiac Is this PR/branch still relevant? Or is everything we actually want already in 2024-beta?

@brettle brettle closed this May 27, 2024
@brettle
brettle deleted the update-phoenix branch May 27, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Change This PR will require an increment to the lib199 major version number

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants