Skip to content

Conversation

@Alexander-Mironenko
Copy link

No description provided.

@rafzip
Copy link
Collaborator

rafzip commented Jan 27, 2026

@DannyPomeranian can you get this ready for testing by tomorrow night?

@DannyPomeranian
Copy link
Contributor

@rafzip I'm working on this now, it should be ready today and will be ready by Thursday

@DannyPomeranian DannyPomeranian marked this pull request as ready for review January 27, 2026 12:16
Alexander-Mironenko

This comment was marked as off-topic.

@auscompgeek
Copy link
Member

@codecov-ai-reviewer review

@auscompgeek
Copy link
Member

@greptileai

@auscompgeek
Copy link
Member

If you're all gonna use AI to review code… at least be upfront about it.

@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

Added hood control functionality to the shooter component using a SparkMax motor controller with an absolute encoder for position feedback.

Key Changes

  • Added hood_motor (SparkMax) with closed-loop PID configuration
  • Implemented change_pitch_relative() and change_pitch_absolute() methods for hood angle control with proper clamping
  • Added feedback methods for monitoring hood angle and setpoint status
  • Added HOOD = 10 to SparkId enum for CAN bus addressing

Issues Found

  • Critical: Hood motor runs at constant speed every execution cycle (line 119) instead of using the configured closed-loop position control (commented out on lines 123-125). This will cause continuous movement regardless of position.
  • Critical: Incorrect math.isclose() usage with rel_tol=5.0 (500% tolerance) instead of abs_tol for absolute degree tolerance
  • Critical: Encoder conversion factor inverted - should convert encoder rotations to degrees, not degrees to rotations
  • Several unused constants (MOTOR_GEAR_RATIO, MOTOR_ROTS_PER_HOOD_DEGREE) and configuration concerns noted in previous threads

The closed-loop position control is intentionally disabled for testing, but the current constant-speed implementation is unsafe for production use.

Confidence Score: 0/5

  • This PR has critical issues that make it unsafe to merge without fixes
  • The hood motor runs continuously at constant speed instead of using position control, which could damage hardware. Additionally, the tolerance check uses incorrect parameters (rel_tol instead of abs_tol) and the encoder conversion factor appears to be inverted.
  • components/shooter.py requires immediate attention - fix the tolerance parameter, encoder conversion factor, and review the motor control strategy before testing

Important Files Changed

Filename Overview
components/shooter.py Added hood motor control with SparkMax, encoder configuration, and pitch control methods, but hood motor runs continuously at constant speed instead of using closed-loop position control

Sequence Diagram

sequenceDiagram
    participant User
    participant ShooterComponent
    participant HoodMotor as Hood Motor (SparkMax)
    participant HoodEncoder as Absolute Encoder
    participant FlywheelMotors as Flywheel Motors
    participant FeederMotor as Feeder Motor

    Note over ShooterComponent: Initialization
    ShooterComponent->>HoodMotor: Configure SparkMax with PID
    ShooterComponent->>HoodEncoder: Set conversion factor & zero offset
    ShooterComponent->>FlywheelMotors: Configure TalonFX gains

    Note over User,ShooterComponent: Control Flow
    User->>ShooterComponent: change_pitch_relative(angle)
    ShooterComponent->>HoodEncoder: getPosition()
    HoodEncoder-->>ShooterComponent: current_angle
    ShooterComponent->>ShooterComponent: clamp(current_angle - angle)
    ShooterComponent->>ShooterComponent: Update desired_hood_angle

    User->>ShooterComponent: change_pitch_absolute(angle)
    ShooterComponent->>ShooterComponent: clamp(90 - angle)
    ShooterComponent->>ShooterComponent: Update desired_hood_angle

    User->>ShooterComponent: shoot()
    ShooterComponent->>ShooterComponent: Set target velocities

    Note over ShooterComponent: Execute Loop (every 20ms)
    ShooterComponent->>FlywheelMotors: VelocityVoltage control
    ShooterComponent->>FeederMotor: PercentOutput control
    ShooterComponent->>HoodMotor: set(hood_move_speed)
    Note over HoodMotor: Runs at constant 0.01 speed<br/>(Closed-loop control commented out)

    ShooterComponent->>HoodEncoder: getPosition()
    HoodEncoder-->>ShooterComponent: current_angle
    ShooterComponent->>ShooterComponent: hood_is_at_setpoint()
    Note over ShooterComponent: Check with math.isclose()<br/>(incorrect rel_tol usage)
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

@DannyPomeranian
Copy link
Contributor

@greptileai

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 93 to 95
def change_pitch_relative(self, angle):
self.desired_hood_angle = self.get_hood_angle() - angle
clamp(self.desired_hood_angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE)
Copy link

Choose a reason for hiding this comment

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

clamp() returns a value but it's not being assigned - desired_hood_angle remains unclamped.

Suggested change
def change_pitch_relative(self, angle):
self.desired_hood_angle = self.get_hood_angle() - angle
clamp(self.desired_hood_angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE)
def change_pitch_relative(self, angle):
self.desired_hood_angle = clamp(
self.get_hood_angle() - angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE
)
Prompt To Fix With AI
This is a comment left during a code review.
Path: components/shooter.py
Line: 93:95

Comment:
`clamp()` returns a value but it's not being assigned - `desired_hood_angle` remains unclamped.

```suggestion
    def change_pitch_relative(self, angle):
        self.desired_hood_angle = clamp(
            self.get_hood_angle() - angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE
        )
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 97 to 99
def change_pitch_absolute(self, angle):
self.desired_hood_angle = 90 - angle
clamp(self.desired_hood_angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE)
Copy link

Choose a reason for hiding this comment

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

clamp() returns a value but it's not being assigned - desired_hood_angle remains unclamped.

Suggested change
def change_pitch_absolute(self, angle):
self.desired_hood_angle = 90 - angle
clamp(self.desired_hood_angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE)
def change_pitch_absolute(self, angle):
self.desired_hood_angle = clamp(
90 - angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE
)
Prompt To Fix With AI
This is a comment left during a code review.
Path: components/shooter.py
Line: 97:99

Comment:
`clamp()` returns a value but it's not being assigned - `desired_hood_angle` remains unclamped.

```suggestion
    def change_pitch_absolute(self, angle):
        self.desired_hood_angle = clamp(
            90 - angle, self.MIN_HOOD_ANGLE, self.MAX_HOOD_ANGLE
        )
```

How can I resolve this? If you propose a fix, please make it concise.


self.feeder_motor.set(ControlMode.PercentOutput, self.target_feeder_percentage)

self.hood_motor.set(self.hood_move_speed)
Copy link

Choose a reason for hiding this comment

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

Hood motor runs continuously at constant speed every execution cycle - will keep moving even when at setpoint or when not commanded.

Prompt To Fix With AI
This is a comment left during a code review.
Path: components/shooter.py
Line: 117:117

Comment:
Hood motor runs continuously at constant speed every execution cycle - will keep moving even when at setpoint or when not commanded.

How can I resolve this? If you propose a fix, please make it concise.

MAX_HOOD_ANGLE = 70 # TODO Tune this value
MIN_HOOD_ANGLE = 10 # TODO Tune this value

MOTOR_GEAR_RATIO = 24 / 22
Copy link

Choose a reason for hiding this comment

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

MOTOR_GEAR_RATIO defined but never used in code.

Prompt To Fix With AI
This is a comment left during a code review.
Path: components/shooter.py
Line: 30:30

Comment:
`MOTOR_GEAR_RATIO` defined but never used in code.

How can I resolve this? If you propose a fix, please make it concise.

@DannyPomeranian
Copy link
Contributor

@greptileai

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

MIN_HOOD_ANGLE = 10 # TODO Tune this value

MOTOR_GEAR_RATIO = 24 / 22
ENCODER_ROTS_PER_HOOD_DEGREE = 54 / 26 / 360
Copy link

Choose a reason for hiding this comment

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

Conversion factor should be in degrees per encoder rotation, not encoder rotations per degree. The formula should be 360 * 26 / 54 (inverse of current value).

Prompt To Fix With AI
This is a comment left during a code review.
Path: components/shooter.py
Line: 31:31

Comment:
Conversion factor should be in degrees per encoder rotation, not encoder rotations per degree. The formula should be `360 * 26 / 54` (inverse of current value).

How can I resolve this? If you propose a fix, please make it concise.

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.

5 participants