Fixes/princeton #23

PhyXTGears-1720 · 2023-03-14T23:19:11Z

-Nick Q

Also add armPose and point header files Yo sontraen onidgon - Emmie Jones

Heeyaw - Emmie J

2D points use 0.0 as their Z axis values VrOOOOoOoOommM - Emmie Jones

Amazing - Emmie Jones

hEehOO Add tHe .0 - Emmie Jones

"Where are your arms?" - Us, 2023 - Emmie Jones

"Remember when you tried to kill me twice?" - Emmie Jones

yeehaw heeyaw - Emmie Jones

"I was chosen for the constitutional convention!" - Emmie Jones

"AAAAAAAAAVVVVVVVVVEEEEEEEEE MARRRIIIIAAAAA" - Emmie Jones

"AAAAAAAA" - Emmie Jones

"" - Emmie Jones

Initial IK only focused on shoulder and elbow joints on a single plane. Add support for turret angle. - Emmie Jones

Use correct method names. add override keyword for compiler checking. add virtual for base class methods. yeehaw - Emmie Jones

Yeeyee - Emmie Jones

(Because Justin said so but doesn't know why) "Double double double" - Emmie Jones

yee - Emmie Jones

weehaw - Emmie Jones

We Are Toast - Emmie Jones

- Add MoveToIntakeCommand. - Add boundary classes. - Add no-go zones. - Define no-go zones. - Add safety points. - Add reach points. - Emmie Jones

Because Justin said so. - Emmie Jones

- Justin C and Emmie Jones

* master: (34 commits) Reformat Fix: Left trigger slows and right trigger speeds up drive Add comments commands: add drivetrain subsystem requirement to DriveTeleopCommand Kill program if unable to open config.toml Fix: build fails when catching exception by value build: Make gradlew executable git: ignore gradle.properties for local overrides add file name to toml load error message change input limiter to accept all values of max and min from the constants file change m_drivetrain to c_drivetrain in driveTeleopCommand add drive command to robot add teleop driving command add subsystems and HIDs to the robot class add Xbox Controller USB interface constants to Interfaces bugfix: change the scope to reset the navX heading to the global scope add enabling/disabling odometry calculation functions add odometry logic add base code for odometry add getter functions for the individual swerve module sensors ...

- Justin C

- Nick Q

Turn wheels to form an X and resist shoves or sliding. - Nick Q

- Justin C

Arm teleop was moving the target position kilometers away due to exhaustion induced programming errors merging two incompatible concepts into an incorrect implementation. Rewrite to focus on one concept: compute various offsets, then add to current target position. - Nick Q and Justin C

When arm teleop command is rescheduled, it will continue moving to the previous target. Add method so the command can be reset to hold at the current arm position, not just at initialization. - Nick Q

Try to avoid further destruction of the arm by reducing the max current limits. - Justin C

- Justin C

-Nick Q

PhyXTGears-1720 · 2023-03-14T23:19:52Z

NOTE: this includes the code from the features/arm branch

-Nick Q

PhyXTGears-1720 · 2023-03-14T23:20:41Z

Added myself because I still need to review arm code

-Nick Q

src/main/cpp/Robot.cpp

src/main/include/Robot.h

src/main/cpp/Robot.cpp

src/main/include/RobotCompileModes.h

nickquednow · 2023-03-22T01:04:25Z

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

+const double k_maxPointSpeed = 0.005;
+const double k_maxPointRotSpeed = (2.0 * M_PI / 10.0) * 0.02;   // radians per second in 20ms.
+const double k_maxWristRotSpeed = (2.0 * M_PI / 2.0) * 0.02;
+const double k_maxGripSpeed = 0.1;


this should go in the Constants.h file

These constants aren't meant to be shared with an other parts of the program. I had concerns that someone might be inclined to reuse them for another purpose and have cause to change them.

I'm inclined to leave this for the competition as it's tested, and change it after.

ok. is there a possibility after competition to rename it to be c_ to clarify that it is not intended to be a global constant?

nickquednow

Most looks good to me. There was only one major thing in there (it involved overloading a function), but other than that, looks good for the arm code.

nickquednow · 2023-03-22T01:06:28Z

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

+        double leftX = c_operatorController->GetLeftX();
+        // Square input to improve fidelity.
+        leftX = DEADZONE(leftX);
+        leftX = std::copysign(std::clamp(leftX * leftX, -1.0, 1.0), leftX);


could we use std::pow(leftX,2) here (consistency).

It's doesn't matter to me. Either way gets the point across. I'd like to think the c++ compiler is smart enough to convert a function call into a single equivalent multiplication, but I couldn't say for sure.

nickquednow · 2023-03-22T01:06:47Z

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

+const double k_maxPointSpeed = 0.005;
+const double k_maxPointRotSpeed = (2.0 * M_PI / 10.0) * 0.02;   // radians per second in 20ms.
+const double k_maxWristRotSpeed = (2.0 * M_PI / 2.0) * 0.02;
+const double k_maxGripSpeed = 0.1;


nickquednow · 2023-03-22T01:09:27Z

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

+        double gripMag = std::sqrt(std::pow(m_target.x,2)+std::pow(m_target.y,2));
+        double gripDir = std::atan2(m_target.x, m_target.y);  // 0 deg == y axis
+
+        // Rotate turret. Speed of rotation is reduced the further the arm reaches.


could we move the second comment to line 44?

nickquednow · 2023-03-22T01:11:27Z

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

+        double rightY = -c_operatorController->GetRightY();    /* Invert so + is up */
+        // Square input to improve fidelity.
+        rightY = DEADZONE(rightY);
+        rightY = std::copysign(std::clamp(rightY * rightY, -1.0, 1.0), rightY);


could we put std::pow(rightY,2) here?

src/main/cpp/commands/arm/ArmTeleopCommand.cpp

src/main/include/subsystems/arm/boundary.h

src/main/include/subsystems/arm/motionPath.h

src/main/include/util/geom.h

boxofrox · 2023-03-22T15:54:25Z

We're three days before a competition. I've got more commits in feature/arm that were tested last night that need to be merged as well.

The concerns here with files related to the arm system will be fixed later in a separate commit. I don't have the time to fix these now, merge this, rebase/merge feature/arm and resolve conflicts due to renames, review to merge feature/arm into master, and test on the robot to make sure nothing broke.

This reverts commit 0f4ec52. combined multiple things into one commit. undoing.

did not realize it was used by the driver, so it was removes. undone :)

StartEndCommand used by autonomous. - Justin C

nickquednow · 2023-03-22T21:38:26Z

Due to the "fix later" issue, I will approve the changes and go ahead and approve the pull request (after the arm fixes have been merged and tested). I will just put the things into a new issue on GH.

nickquednow

There was only 1 odd thing I saw in robot.h other than that, it looks good.

src/main/include/Robot.h

boxofrox · 2023-03-23T02:37:07Z

Due to the "fix later" issue, I will approve the changes and go ahead and approve the pull request (after the arm fixes have been merged and tested). I will just put the things into a new issue on GH.

I already opened an issue referencing this. All of the individual "will fix later" is addressed in the feature/arm branch, which I've since merged. The stuff that I didn't mark in here as "will fix later" still needs to be addressed.

PhyXTGears-1720 added 30 commits January 28, 2023 14:29

Add arm.h w/ some IK math and arm length consts

38a3d5b

Also add armPose and point header files Yo sontraen onidgon - Emmie Jones

Add #include for cmath

ab6549d

Heeyaw - Emmie J

Make Point class take Z param; update 2D points

89b32c6

2D points use 0.0 as their Z axis values VrOOOOoOoOommM - Emmie Jones

Rename "get*Pos" Functions to "calc*Pos" for clarity

a26cbf1

Amazing - Emmie Jones

Add .0 to arm consts to denote float type

b1d6c12

hEehOO Add tHe .0 - Emmie Jones

Add empty functions for reading arm angles

f4badca

"Where are your arms?" - Us, 2023 - Emmie Jones

Add functions for getting measured angles on arm

ca8bf50

"Remember when you tried to kill me twice?" - Emmie Jones

Add Boundary class for arm movement safety

3ee54bd

yeehaw heeyaw - Emmie Jones

Add empty set__Angle arm functions (and add diagram)

c89d6b1

"I was chosen for the constitutional convention!" - Emmie Jones

Rename getWristAngle() to getWristRollAngle() for consistency

037c033

"AAAAAAAAAVVVVVVVVVEEEEEEEEE MARRRIIIIAAAAA" - Emmie Jones

Add forgotten Pragma Once to arm custom classes

e26986e

"AAAAAAAA" - Emmie Jones

Factor in Turret in calcIKJointPoses()

083ce40

"" - Emmie Jones

Update inverse kinematics for 3 dimensions

0a8c1c4

Initial IK only focused on shoulder and elbow joints on a single plane. Add support for turret angle. - Emmie Jones

Merge branch 'master' into feature/arm

a76d1fd

Fix boundary code

23efaf1

Use correct method names. add override keyword for compiler checking. add virtual for base class methods. yeehaw - Emmie Jones

Add constructor for NotBoundary

830a75b

Yeeyee - Emmie Jones

Switched from floats to doubles

950b6f3

(Because Justin said so but doesn't know why) "Double double double" - Emmie Jones

Fix constants prefix

77b2175

yee - Emmie Jones

Fix arguments for calcElbowPos()

2d5d196

weehaw - Emmie Jones

Switch m prefix to m_

458d3af

We Are Toast - Emmie Jones

Mass dump of arm subsystem updates

a726c28

- Add MoveToIntakeCommand. - Add boundary classes. - Add no-go zones. - Define no-go zones. - Add safety points. - Add reach points. - Emmie Jones

Convert Point to use double instead of float

e032a90

Because Justin said so. - Emmie Jones

Fix headers

06d03b5

- Justin C and Emmie Jones

Add incomplete implementation of motion path.

08bbd47

- Justin C and Emmie Jones

Add config limits and setpoints for arm subsystem

346aeea

- Justin C

Apply conversion factors and zero offsets when reading sensors.

f798087

- Justin C

Clamp values when adjust angles and distances.

162a946

- Justin C

Rename vars: Ang -> Angle

0340e64

- Justin C

arm: Add method moveToPoint()

543ec57

- Justin C

PhyXTGears-1720 added 10 commits March 12, 2023 13:45

auto: simple auto scores one cube in hybrid

dea32f1

- Nick Q

drive: Add lock movement to drive X button

9aeec2f

Turn wheels to form an X and resist shoves or sliding. - Nick Q

arm: Prevent arm movement by moving to current location

6c694a9

- Justin C

arm: Add reset target method to arm teleop command

4c9a09f

When arm teleop command is rescheduled, it will continue moving to the previous target. Add method so the command can be reset to hold at the current arm position, not just at initialization. - Nick Q

arm: Set current limits on arm motor

e794f08

Try to avoid further destruction of the arm by reducing the max current limits. - Justin C

arm: Calibrate limits and zeroes

1b56539

- Justin C

remove smartdashboard debugging output

d1b012c

-Nick Q

remove competition define headers for the simple auto

f67f409

-Nick Q

change code to align with coding guidelines

0f4ec52

-Nick Q

PhyXTGears-1720 requested a review from boxofrox March 14, 2023 23:19

PhyXTGears-1720 requested a review from nickquednow March 14, 2023 23:20

boxofrox requested changes Mar 21, 2023

View reviewed changes

src/main/cpp/Robot.cpp Outdated Show resolved Hide resolved

src/main/include/Robot.h Outdated Show resolved Hide resolved

src/main/cpp/Robot.cpp Outdated Show resolved Hide resolved

src/main/include/RobotCompileModes.h Show resolved Hide resolved

nickquednow reviewed Mar 22, 2023

View reviewed changes

nickquednow requested changes Mar 22, 2023

View reviewed changes

boxofrox mentioned this pull request Mar 22, 2023

Resolve concerns from PR #23 review #24

Open

nickquednow added 5 commits March 22, 2023 12:25

Revert "change code to align with coding guidelines"

a335343

This reverts commit 0f4ec52. combined multiple things into one commit. undoing.

change compile mode to debug

fe571bf

remove unneeded headers in robot.h

d002d5f

re-add driverstation field centric output

e5009ac

did not realize it was used by the driver, so it was removes. undone :)

rename c_simpleSuto to c_autoDumpCubeAndScore for clarity

2bbe6eb

boxofrox approved these changes Mar 22, 2023

View reviewed changes

Restore missing header import

a907c65

StartEndCommand used by autonomous. - Justin C

nickquednow approved these changes Mar 22, 2023

View reviewed changes

src/main/include/Robot.h Show resolved Hide resolved

PhyXTGears-1720 merged commit 2f5c48e into master Mar 22, 2023

Fixes/princeton #23

Fixes/princeton #23

Uh oh!

Conversation

PhyXTGears-1720 commented Mar 14, 2023

Uh oh!

PhyXTGears-1720 commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PhyXTGears-1720 commented Mar 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickquednow Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickquednow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

boxofrox commented Mar 22, 2023

Uh oh!

nickquednow commented Mar 22, 2023

Uh oh!

nickquednow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

boxofrox commented Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PhyXTGears-1720 commented Mar 14, 2023 •

edited

Loading

nickquednow Mar 22, 2023 •

edited

Loading