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

Fixing imports and adding Encoder printouts to the ivp #7

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

Conversation

ksiegall
Copy link
Member

In the code currently stored on main, importing from WPILib.py fails due to an import failure. This pull request fixes that and also adds the encoder printouts to the installation verification program, which will enable users to test if their motors are oriented properly.

@ksiegall ksiegall requested a review from bradamiller March 27, 2023 20:29
@@ -55,13 +55,16 @@ def ivp():
while not buttons.is_GP20_pressed() and not buttons.is_GP21_pressed():
print(f"Ultrasonic Distance: {sonar.get_distance()}")
time.sleep(0.1)
while (buttons.is_GP20_pressed() or buttons.is_GP21_pressed()):
time.sleep(.01)
wait_for_button()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This waits for press + released now, is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intentional originally, but upon thinking about it a little more, I realized the initial implementation was correct. I've reverted that change and added comments for clarity

WPILib/WPILib.py Outdated
from _servo import Servo
from _buttons import Buttons
from _led import RGBLED
from . import _drivetrain

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not do the following and avoid needing to change the code below? Is it a limitation of circuitpython?

from ._drivetrain import Drivetrain

Local imports: https://peps.python.org/pep-0008/#imports

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing wrong with your recommendation, just that I'm not a primary python developer so I didn't know the exact syntax, and thus reverted to what I did know.

I've implemented your suggestion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 both options work, I was just curious.

code.py Outdated
#
# Your Code Here!
#
pass

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this default to ivp() until the user edits it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should. I've fixed it in one of the recent commits.

@ksiegall
Copy link
Member Author

I sped up the LED testing code that runs during the IVP as it was taking 6 seconds to cycle through a couple different color/brightness combinations, which is a lot of time to spend on testing a non-essential function of the board. I've reduced this time to 2 seconds.

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.

2 participants