Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added example SwerveControllerCommand #108

Open
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

chaoticthegreat
Copy link

No description provided.

@chaoticthegreat
Copy link
Author

Added docstrings manually, for some reason whenever I run the code I get an error saying "No module named commands2" even though I have installed the library but the code looks good.

SwerveControllerCommand/Subsystems/DriveSubsystem.py Outdated Show resolved Hide resolved
SwerveControllerCommand/robot.py Outdated Show resolved Hide resolved
SwerveControllerCommand/robot.py Outdated Show resolved Hide resolved
@virtuald
Copy link
Member

virtuald commented Jan 9, 2024

If you're getting an error regarding commands2 not being found, then it's not installed for the current python interpreter (python -m pip list will tell you if robotpy-commands-v2 is installed). If it shows in the pip list for the current interpreter, that would be interesting.

This adds your robot.py file to the tests folder
@Suave101
Copy link
Contributor

Suave101 commented Jan 9, 2024

Could you please place the example within the "tests" folder in the repository? This helps maintain our standard project structure. To help move things along, I've already created a pull request to add the example to the tests folder. You can check it out here. ✨ Would you also be able to format the code using Black, as outlined in our contribution guide? This helps keep our codebase consistent and readable for everyone. You can format your code with black with this took I like to use called Black Playground

chaoticthegreat and others added 2 commits January 9, 2024 15:12
Add robot.py file to tests
…docstrings, changed TimedRobot to TimedCommandRobot
@chaoticthegreat
Copy link
Author

Thank you for the pull request, I have finally got commands2 to work, and have fixed the issues outlined. Just to recap, I have lowercased the folders and files, formatted all the code with black using Black Playground, adjusted robot.py for TimedCommandRobot instead of TimedRobot, added a few docstrings and changed a few into comments.

@auscompgeek
Copy link
Member

This PR is still not formatted with black. I would highly recommend setting up black in your IDE.

The file paths still need to be fixed to lowercase.

@chaoticthegreat
Copy link
Author

chaoticthegreat commented Jan 10, 2024

@auscompgeek Sorry about the files being renamed, git was being weird. I did paste all my files into the black playground, but I have now ran it in my IDE.

@virtuald
Copy link
Member

I wouldn't recommend copy/pasting into the black playground... that's way too much effort.

Just configure vscode to use black, and it will install it for you. Or, pip install black, then run it from the command line. Either of those will be much less error prone.

@chaoticthegreat
Copy link
Author

Ye did that.

@chaoticthegreat
Copy link
Author

Within drivesubsystems.py and swervemodule.py I can't import constants. How would I fix this? Most solution just say to modify sys.path but the robotpy porting guidelines says not to.

@chaoticthegreat
Copy link
Author

@auscompgeek I fixed most of the errors, but I get an error saying that commands2 has no attribute named Swerve4ControllerCommand, when the docs clearly says it does. I get this error from running the workflow file from this repo.

SwerveControllerCommand/subsystems/swervemodule.py Outdated Show resolved Hide resolved
SwerveControllerCommand/subsystems/drivesubsystems.py Outdated Show resolved Hide resolved
SwerveControllerCommand/robotcontainer.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants