Open
Conversation
the robot doesnt work in simulation and the abstractions are causing trouble so
the robot doesnt work in simulation and the abstractions are causing trouble so
Baconing
added
the
Status/Blocked
WalkthroughThis PR removes the IO abstraction pattern across multiple subsystems (Collector, Shooter, Swerve Drive, Turntable, Vision) and eliminates related logging infrastructure. Subsystems now receive direct motor and camera dependencies instead of IO interfaces. The replayWatch Gradle task is also removed, and teleop drive command wiring is updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/main/kotlin/org/frc5183/robot/subsystems/shooter/ShooterSubsystem.kt (1)
7-11: Consider making motor properties private for encapsulation.Exposing
shooter,intake, andfeederas publicvalproperties allows external code to bypass the subsystem's control methods and directly manipulate the motors. In FRC's command-based architecture, this could lead to command conflicts.♻️ Suggested change to improve encapsulation
class ShooterSubsystem( - val shooter: SparkMax, - val intake: SparkMax, - val feeder: SparkMax, + private val shooter: SparkMax, + private val intake: SparkMax, + private val feeder: SparkMax, ) : SubsystemBase() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/frc5183/robot/subsystems/shooter/ShooterSubsystem.kt` around lines 7 - 11, The ShooterSubsystem currently exposes SparkMax instances as public vals (shooter, intake, feeder); make these properties private (e.g., change to private val shooter/intake/feeder) to enforce encapsulation and prevent external commands from directly manipulating motor controllers, and if external access is required expose only controlled methods or read-only status getters on ShooterSubsystem (e.g., getShooterVelocity/getIntakeState) so all motor actions go through the subsystem's control methods.src/main/kotlin/org/frc5183/robot/commands/collector/RaiseCollector.kt (1)
5-5: Unused import:ShooterSubsystem.This import is not used in the file.
♻️ Proposed fix
-import org.frc5183.robot.subsystems.shooter.ShooterSubsystem🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/frc5183/robot/commands/collector/RaiseCollector.kt` at line 5, Remove the unused import of org.frc5183.robot.subsystems.shooter.ShooterSubsystem from RaiseCollector.kt; open the RaiseCollector class/file and delete the import line referencing ShooterSubsystem so only necessary imports remain and the file compiles without unused imports.src/main/kotlin/org/frc5183/robot/constants/Controls.kt (1)
6-8: Unused imports:TeleopDriveCommandandSwerveInputStream.These imports appear to be leftover from the refactor and are no longer used.
♻️ Proposed cleanup
-import org.frc5183.robot.commands.drive.TeleopDriveCommand -import swervelib.SwerveInputStream🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/frc5183/robot/constants/Controls.kt` around lines 6 - 8, The file imports TeleopDriveCommand and SwerveInputStream but neither symbol is referenced; remove the unused imports (the lines importing TeleopDriveCommand and SwerveInputStream) from Controls.kt to clean up the code and eliminate the unused-import warnings, leaving only necessary imports such as SwerveDriveSubsystem.src/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.kt (1)
60-62: Consider making helper functionsprivate.
speedIsUpandspeedIsDownappear to be internal implementation details. Making themprivatewould reduce the public API surface.♻️ Proposed refactor
- fun speedIsUp(speed: Double): Boolean = speed.sign == 1.0 + private fun speedIsUp(speed: Double): Boolean = speed.sign == 1.0 - fun speedIsDown(speed: Double): Boolean = speed.sign == -1.0 + private fun speedIsDown(speed: Double): Boolean = speed.sign == -1.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.kt` around lines 60 - 62, The functions speedIsUp and speedIsDown are internal helpers exposed publicly; make them private to reduce the public API surface by changing their visibility to private (i.e., mark functions speedIsUp(speed: Double) and speedIsDown(speed: Double) as private) so they remain available inside CollectorSubsystem but are not part of the class's public interface.src/main/kotlin/org/frc5183/robot/commands/collector/IntakeCommand.kt (1)
5-5: Unused import:ShooterSubsystem.This import is not used in the file.
♻️ Proposed fix
-import org.frc5183.robot.subsystems.shooter.ShooterSubsystem🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/frc5183/robot/commands/collector/IntakeCommand.kt` at line 5, Remove the unused import of ShooterSubsystem from IntakeCommand.kt; locate the import line importing org.frc5183.robot.subsystems.shooter.ShooterSubsystem at the top of the file (in the IntakeCommand class/file) and delete it so only actually used imports remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/org/frc5183/robot/commands/collector/LowerCollector.kt`:
- Line 5: The import of ShooterSubsystem at the top of LowerCollector is unused;
open the LowerCollector class and remove the line importing
org.frc5183.robot.subsystems.shooter.ShooterSubsystem so the file no longer
contains an unused import (leave all other imports and the LowerCollector class
unchanged).
In `@src/main/kotlin/org/frc5183/robot/constants/Constants.kt`:
- Around line 5-9: The three vals in the Constants object (COLLECTOR_ARM_TOP,
COLLECTOR_ARM_BOTTOM, COLLECTOR_ARM_DELTA) are declared without initializers and
will not compile; fix by assigning each an appropriate Angle value (e.g., using
Units to convert degrees/radians as required) and add the import
edu.wpi.first.units.Units at the top so you can construct the Angle constants
(update the Constants object to initialize COLLECTOR_ARM_TOP,
COLLECTOR_ARM_BOTTOM, and COLLECTOR_ARM_DELTA with concrete Angle expressions).
In `@src/main/kotlin/org/frc5183/robot/constants/Controls.kt`:
- Around line 15-20: The registerControls function is setting
drive.defaultCommand to an undefined symbol TELEOP_DRIVE_COMMAND; replace that
reference with the declared property DRIVE_COMMAND so the code compiles (update
the assignment in registerControls where drive.defaultCommand is set to use
DRIVE_COMMAND instead of TELEOP_DRIVE_COMMAND).
In
`@src/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.kt`:
- Around line 38-39: The periodic safety check in CollectorSubsystem incorrectly
uses speedIsUp(armSpeed) for the bottom limit; change the condition that
currently reads if (atBottom && speedIsUp(armSpeed)) stopArm() to use
speedIsDown(armSpeed) instead so the arm stops when it's trying to move down
into the bottom limit; locate this logic in the CollectorSubsystem class where
atTop, atBottom, armSpeed, speedIsUp/speedIsDown and stopArm are referenced and
update the predicate accordingly.
In
`@src/main/kotlin/org/frc5183/robot/subsystems/turntable/TurntableSubsystem.kt`:
- Around line 13-22: The targets getter currently calls camera.allUnreadResults
which is destructive and periodic() logs via targets, draining the camera queue
before AlignTurntable.execute() can read them; to fix, add a backing field
(e.g., cachedTargets: Array<PhotonTrackedTarget>) and update it once at the
start of each periodic() by calling camera.allUnreadResults (or use
FixedCamera.updateUnreadResults() pattern), then have the targets property
return cachedTargets so subsequent calls (including AlignTurntable.execute()) in
the same cycle see the same non-destructive snapshot.
---
Nitpick comments:
In `@src/main/kotlin/org/frc5183/robot/commands/collector/IntakeCommand.kt`:
- Line 5: Remove the unused import of ShooterSubsystem from IntakeCommand.kt;
locate the import line importing
org.frc5183.robot.subsystems.shooter.ShooterSubsystem at the top of the file (in
the IntakeCommand class/file) and delete it so only actually used imports
remain.
In `@src/main/kotlin/org/frc5183/robot/commands/collector/RaiseCollector.kt`:
- Line 5: Remove the unused import of
org.frc5183.robot.subsystems.shooter.ShooterSubsystem from RaiseCollector.kt;
open the RaiseCollector class/file and delete the import line referencing
ShooterSubsystem so only necessary imports remain and the file compiles without
unused imports.
In `@src/main/kotlin/org/frc5183/robot/constants/Controls.kt`:
- Around line 6-8: The file imports TeleopDriveCommand and SwerveInputStream but
neither symbol is referenced; remove the unused imports (the lines importing
TeleopDriveCommand and SwerveInputStream) from Controls.kt to clean up the code
and eliminate the unused-import warnings, leaving only necessary imports such as
SwerveDriveSubsystem.
In
`@src/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.kt`:
- Around line 60-62: The functions speedIsUp and speedIsDown are internal
helpers exposed publicly; make them private to reduce the public API surface by
changing their visibility to private (i.e., mark functions speedIsUp(speed:
Double) and speedIsDown(speed: Double) as private) so they remain available
inside CollectorSubsystem but are not part of the class's public interface.
In `@src/main/kotlin/org/frc5183/robot/subsystems/shooter/ShooterSubsystem.kt`:
- Around line 7-11: The ShooterSubsystem currently exposes SparkMax instances as
public vals (shooter, intake, feeder); make these properties private (e.g.,
change to private val shooter/intake/feeder) to enforce encapsulation and
prevent external commands from directly manipulating motor controllers, and if
external access is required expose only controlled methods or read-only status
getters on ShooterSubsystem (e.g., getShooterVelocity/getIntakeState) so all
motor actions go through the subsystem's control methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43a90ba5-d0d3-463e-bac0-e9d3adf46cd1
📒 Files selected for processing (32)
build.gradle.ktssrc/main/kotlin/org/frc5183/robot/Robot.ktsrc/main/kotlin/org/frc5183/robot/commands/collector/DriveCollector.ktsrc/main/kotlin/org/frc5183/robot/commands/collector/IntakeCommand.ktsrc/main/kotlin/org/frc5183/robot/commands/collector/LowerCollector.ktsrc/main/kotlin/org/frc5183/robot/commands/collector/RaiseCollector.ktsrc/main/kotlin/org/frc5183/robot/constants/Constants.ktsrc/main/kotlin/org/frc5183/robot/constants/Controls.ktsrc/main/kotlin/org/frc5183/robot/logging/AutoLogInputs.ktsrc/main/kotlin/org/frc5183/robot/logging/struct/EstimatedRobotPoseStruct.ktsrc/main/kotlin/org/frc5183/robot/logging/struct/PhotonTrackedTargetStruct.ktsrc/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.ktsrc/main/kotlin/org/frc5183/robot/subsystems/collector/io/CollectorIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/collector/io/CollectorIOInputs.ktsrc/main/kotlin/org/frc5183/robot/subsystems/collector/io/RealCollectorIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/drive/SwerveDriveSubsystem.ktsrc/main/kotlin/org/frc5183/robot/subsystems/drive/io/RealSwerveDriveIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/drive/io/SimulatedSwerveDriveIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/drive/io/SwerveDriveIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/drive/io/SwerveDriveIOInputs.ktsrc/main/kotlin/org/frc5183/robot/subsystems/shooter/ShooterSubsystem.ktsrc/main/kotlin/org/frc5183/robot/subsystems/shooter/io/RealShooterIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/shooter/io/ShooterIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/shooter/io/ShooterIOInputs.ktsrc/main/kotlin/org/frc5183/robot/subsystems/turntable/TurntableSubsystem.ktsrc/main/kotlin/org/frc5183/robot/subsystems/turntable/io/RealTurntableIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/turntable/io/TurntableIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/turntable/io/TurntableIOInputs.ktsrc/main/kotlin/org/frc5183/robot/subsystems/vision/VisionSubsystem.ktsrc/main/kotlin/org/frc5183/robot/subsystems/vision/io/RealVisionIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/vision/io/VisionIO.ktsrc/main/kotlin/org/frc5183/robot/subsystems/vision/io/VisionIOInputs.kt
💤 Files with no reviewable changes (20)
- build.gradle.kts
- src/main/kotlin/org/frc5183/robot/subsystems/vision/io/RealVisionIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/collector/io/CollectorIOInputs.kt
- src/main/kotlin/org/frc5183/robot/subsystems/collector/io/CollectorIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/shooter/io/ShooterIOInputs.kt
- src/main/kotlin/org/frc5183/robot/subsystems/vision/io/VisionIO.kt
- src/main/kotlin/org/frc5183/robot/logging/struct/EstimatedRobotPoseStruct.kt
- src/main/kotlin/org/frc5183/robot/subsystems/drive/io/SwerveDriveIOInputs.kt
- src/main/kotlin/org/frc5183/robot/logging/AutoLogInputs.kt
- src/main/kotlin/org/frc5183/robot/subsystems/turntable/io/RealTurntableIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/drive/io/RealSwerveDriveIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/shooter/io/RealShooterIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/turntable/io/TurntableIOInputs.kt
- src/main/kotlin/org/frc5183/robot/subsystems/drive/io/SwerveDriveIO.kt
- src/main/kotlin/org/frc5183/robot/logging/struct/PhotonTrackedTargetStruct.kt
- src/main/kotlin/org/frc5183/robot/subsystems/shooter/io/ShooterIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/turntable/io/TurntableIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/collector/io/RealCollectorIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/drive/io/SimulatedSwerveDriveIO.kt
- src/main/kotlin/org/frc5183/robot/subsystems/vision/io/VisionIOInputs.kt
|
|
||
| import edu.wpi.first.wpilibj2.command.Command | ||
| import org.frc5183.robot.subsystems.collector.CollectorSubsystem | ||
| import org.frc5183.robot.subsystems.shooter.ShooterSubsystem |
There was a problem hiding this comment.
Remove unused import.
ShooterSubsystem is imported but never used in this class.
🧹 Proposed fix
import edu.wpi.first.wpilibj2.command.Command
import org.frc5183.robot.subsystems.collector.CollectorSubsystem
-import org.frc5183.robot.subsystems.shooter.ShooterSubsystem📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.frc5183.robot.subsystems.shooter.ShooterSubsystem | |
| import edu.wpi.first.wpilibj2.command.Command | |
| import org.frc5183.robot.subsystems.collector.CollectorSubsystem |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/frc5183/robot/commands/collector/LowerCollector.kt` at
line 5, The import of ShooterSubsystem at the top of LowerCollector is unused;
open the LowerCollector class and remove the line importing
org.frc5183.robot.subsystems.shooter.ShooterSubsystem so the file no longer
contains an unused import (leave all other imports and the LowerCollector class
unchanged).
Comment on lines
+5
to
+9
| object Constants { | ||
| val COLLECTOR_ARM_TOP: Angle | ||
| val COLLECTOR_ARM_BOTTOM: Angle | ||
| val COLLECTOR_ARM_DELTA: Angle | ||
| } |
There was a problem hiding this comment.
Uninitialized val properties will cause compilation error.
In Kotlin, val properties within an object declaration must be initialized. These three properties lack initializers, which will fail compilation.
🐛 Proposed fix: Initialize the constants with appropriate values
object Constants {
- val COLLECTOR_ARM_TOP: Angle
- val COLLECTOR_ARM_BOTTOM: Angle
- val COLLECTOR_ARM_DELTA: Angle
+ val COLLECTOR_ARM_TOP: Angle = Units.Rotations.of(0.0) // TODO: Set actual top position
+ val COLLECTOR_ARM_BOTTOM: Angle = Units.Rotations.of(0.0) // TODO: Set actual bottom position
+ val COLLECTOR_ARM_DELTA: Angle = Units.Rotations.of(0.01) // TODO: Set actual tolerance
}You'll need to add the import:
import edu.wpi.first.units.Units🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/frc5183/robot/constants/Constants.kt` around lines 5 - 9,
The three vals in the Constants object (COLLECTOR_ARM_TOP, COLLECTOR_ARM_BOTTOM,
COLLECTOR_ARM_DELTA) are declared without initializers and will not compile; fix
by assigning each an appropriate Angle value (e.g., using Units to convert
degrees/radians as required) and add the import edu.wpi.first.units.Units at the
top so you can construct the Angle constants (update the Constants object to
initialize COLLECTOR_ARM_TOP, COLLECTOR_ARM_BOTTOM, and COLLECTOR_ARM_DELTA with
concrete Angle expressions).
Comment on lines
+15
to
20
| lateinit var DRIVE_COMMAND: Command | ||
|
|
||
| fun registerControls(drive: SwerveDriveSubsystem) { | ||
| CommandScheduler.getInstance().activeButtonLoop.clear() | ||
|
|
||
| TELEOP_DRIVE_COMMAND = | ||
| TeleopDriveCommand( | ||
| drive, | ||
| xInput = { if (DRIVER_CONTROLLER.leftX.absoluteValue < .2) 0.0 else DRIVER_CONTROLLER.leftX }, | ||
| yInput = { if (DRIVER_CONTROLLER.leftY.absoluteValue < .2) 0.0 else DRIVER_CONTROLLER.leftY }, | ||
| rotationInput = { 0.0 }, | ||
| fieldRelative = false, | ||
| ) | ||
|
|
||
| drive.defaultCommand = TELEOP_DRIVE_COMMAND |
There was a problem hiding this comment.
TELEOP_DRIVE_COMMAND is undefined; should use DRIVE_COMMAND.
Line 20 references TELEOP_DRIVE_COMMAND which doesn't exist. The declared property is DRIVE_COMMAND. This will fail compilation.
🐛 Proposed fix
- drive.defaultCommand = TELEOP_DRIVE_COMMAND
+ drive.defaultCommand = DRIVE_COMMAND📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| lateinit var DRIVE_COMMAND: Command | |
| fun registerControls(drive: SwerveDriveSubsystem) { | |
| CommandScheduler.getInstance().activeButtonLoop.clear() | |
| TELEOP_DRIVE_COMMAND = | |
| TeleopDriveCommand( | |
| drive, | |
| xInput = { if (DRIVER_CONTROLLER.leftX.absoluteValue < .2) 0.0 else DRIVER_CONTROLLER.leftX }, | |
| yInput = { if (DRIVER_CONTROLLER.leftY.absoluteValue < .2) 0.0 else DRIVER_CONTROLLER.leftY }, | |
| rotationInput = { 0.0 }, | |
| fieldRelative = false, | |
| ) | |
| drive.defaultCommand = TELEOP_DRIVE_COMMAND | |
| lateinit var DRIVE_COMMAND: Command | |
| fun registerControls(drive: SwerveDriveSubsystem) { | |
| CommandScheduler.getInstance().activeButtonLoop.clear() | |
| drive.defaultCommand = DRIVE_COMMAND |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/frc5183/robot/constants/Controls.kt` around lines 15 -
20, The registerControls function is setting drive.defaultCommand to an
undefined symbol TELEOP_DRIVE_COMMAND; replace that reference with the declared
property DRIVE_COMMAND so the code compiles (update the assignment in
registerControls where drive.defaultCommand is set to use DRIVE_COMMAND instead
of TELEOP_DRIVE_COMMAND).
Comment on lines
+38
to
+39
| if (atTop && speedIsUp(armSpeed)) stopArm() | ||
| if (atBottom && speedIsUp(armSpeed)) stopArm() |
There was a problem hiding this comment.
Logic error: Line 39 should check speedIsDown, not speedIsUp.
The periodic safety check at line 39 uses speedIsUp(armSpeed) but should use speedIsDown(armSpeed). When at the bottom, the arm should stop if it's still trying to move down, not up.
🐛 Proposed fix
if (atTop && speedIsUp(armSpeed)) stopArm()
- if (atBottom && speedIsUp(armSpeed)) stopArm()
+ if (atBottom && speedIsDown(armSpeed)) stopArm()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (atTop && speedIsUp(armSpeed)) stopArm() | |
| if (atBottom && speedIsUp(armSpeed)) stopArm() | |
| if (atTop && speedIsUp(armSpeed)) stopArm() | |
| if (atBottom && speedIsDown(armSpeed)) stopArm() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/frc5183/robot/subsystems/collector/CollectorSubsystem.kt`
around lines 38 - 39, The periodic safety check in CollectorSubsystem
incorrectly uses speedIsUp(armSpeed) for the bottom limit; change the condition
that currently reads if (atBottom && speedIsUp(armSpeed)) stopArm() to use
speedIsDown(armSpeed) instead so the arm stops when it's trying to move down
into the bottom limit; locate this logic in the CollectorSubsystem class where
atTop, atBottom, armSpeed, speedIsUp/speedIsDown and stopArm are referenced and
update the predicate accordingly.
Comment on lines
13
to
+22
| val targets: Array<PhotonTrackedTarget> | ||
| get() = ioInputs.targets | ||
| get() = | ||
| camera.allUnreadResults | ||
| .map { it.targets } | ||
| .flatten() | ||
| .toTypedArray() | ||
|
|
||
| override fun periodic() { | ||
| io.updateInputs(ioInputs) | ||
| Logger.recordOutput("Turntable/Speed", motor.get()) | ||
| Logger.recordOutput("Turntable/Targets", targets.map { it.fiducialId }.toIntArray()) |
There was a problem hiding this comment.
allUnreadResults is destructive; periodic() drains the queue before commands can read targets.
PhotonCamera.allUnreadResults returns and removes results from the internal queue. Since targets is a computed property that calls allUnreadResults on every access:
periodic()callstargetson line 22 for logging- This drains the camera results queue
- When
AlignTurntable.execute()later callsturntable.targets, it receives an empty array
Cache the results once per cycle, similar to FixedCamera.updateUnreadResults():
🐛 Proposed fix: cache results in periodic()
class TurntableSubsystem(
val motor: SparkMax,
val camera: PhotonCamera,
) : SubsystemBase() {
- val targets: Array<PhotonTrackedTarget>
- get() =
- camera.allUnreadResults
- .map { it.targets }
- .flatten()
- .toTypedArray()
+ var targets: Array<PhotonTrackedTarget> = emptyArray()
+ private set
override fun periodic() {
+ targets = camera.allUnreadResults
+ .flatMap { it.targets }
+ .toTypedArray()
+
Logger.recordOutput("Turntable/Speed", motor.get())
Logger.recordOutput("Turntable/Targets", targets.map { it.fiducialId }.toIntArray())
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/frc5183/robot/subsystems/turntable/TurntableSubsystem.kt`
around lines 13 - 22, The targets getter currently calls camera.allUnreadResults
which is destructive and periodic() logs via targets, draining the camera queue
before AlignTurntable.execute() can read them; to fix, add a backing field
(e.g., cachedTargets: Array<PhotonTrackedTarget>) and update it once at the
start of each periodic() by calling camera.allUnreadResults (or use
FixedCamera.updateUnreadResults() pattern), then have the targets property
return cachedTargets so subsequent calls (including AlignTurntable.execute()) in
the same cycle see the same non-destructive snapshot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
blocked by #18
Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores