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

New climb #167

Merged
merged 18 commits into from
Feb 12, 2025
Merged

New climb #167

merged 18 commits into from
Feb 12, 2025

Conversation

Emma03L
Copy link
Contributor

@Emma03L Emma03L commented Feb 9, 2025

No description provided.

@Emma03L Emma03L requested review from katzuv and rakrakon February 9, 2025 11:32
@Emma03L Emma03L self-assigned this Feb 9, 2025
@Emma03L Emma03L added the subsystem Pull request for a subsystem label Feb 9, 2025
Copy link

github-actions bot commented Feb 9, 2025

Qodana Community for JVM

225 new problems were found

Inspection name Severity Problems
Unused symbol 🔶 Warning 125
Unnecessary modifier 🔶 Warning 22
Constant conditions 🔶 Warning 6
Field can be local 🔶 Warning 5
Unnecessary conversion to 'String' 🔶 Warning 5
Unused receiver parameter 🔶 Warning 4
Declaration has problems in Javadoc references 🔶 Warning 3
Unresolved reference in KDoc 🔶 Warning 3
Field may be 'final' 🔶 Warning 2
Busy wait 🔶 Warning 1
Infinite loop statement 🔶 Warning 1
Main function should return 'Unit' 🔶 Warning 1
Mismatched read and write of array 🔶 Warning 1
'size() == 0' can be replaced with 'isEmpty()' 🔶 Warning 1
Call to 'printStackTrace()' 🔶 Warning 1
Unused import 🔶 Warning 1
Class member can have 'private' visibility ◽️ Notice 18
Constant values ◽️ Notice 12
Lambda argument inside parentheses ◽️ Notice 11
Commented out code ◽️ Notice 1
Method can be extracted ◽️ Notice 1

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

Copy link
Member

@katzuv katzuv left a comment

Choose a reason for hiding this comment

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

I think the subsystem was simplified too much. IMO it's very important to have a very fast climber. If we can climb (reliably) in a few seconds it will guarantee us a PR, and the "fastness" will increase the chance we climb even if we got stuck somewhere in the field, the driver took a few more seconds to align to the cage, etc.
I would bring back the:

  • Second motor: if there is enough weight, the climber will be faster and the center of gravity will be lower.
  • Stopper: this will ensure we get the climb RP even if we climbed very little. However, we need to check how much height the robot loses in 3 seconds, at a very low angle (there the moment of gravity is the largest).
  • CANCoder: If the subsystem will be power based, we might need to put a low value at the beginning and end of the movement, so to not strain the motor to much and/or hit the cage. Bringing back the CANcoder will make position control possible, and thus probably decreasing the time it takes to climb.

SensorToMechanismRatio = SENSOR_TO_MECHANISM
FeedbackRemoteSensorID = CANCODER_ID
FeedbackSensorSource = FeedbackSensorSourceValue.FusedCANcoder
}
SoftwareLimitSwitch.apply {
Copy link
Member

@katzuv katzuv Feb 9, 2025

Choose a reason for hiding this comment

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

If there is no CANcoder now, I don't know how reliable the internal encoder will be, so perhaps the software limits should be removed.
Why, though, was the CANcoder removed? It will help us make the climb faster, which is important for a last-second climb (and RP).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did remove the soft limits but didn't commit it because we're not sure if there will be a cancoder

@Emma03L Emma03L merged commit 2fbbbb9 into dev Feb 12, 2025
3 checks passed
@Emma03L Emma03L deleted the feature/new-climb branch February 12, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem Pull request for a subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants