Skip to content
This repository was archived by the owner on Aug 24, 2025. It is now read-only.

Conversation

@CodingJackalNoob
Copy link

1st subsystem code! pls be harsh

Copy link
Member

@00magikarp 00magikarp left a comment

Choose a reason for hiding this comment

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

just wanna say that for ur first subsystem this is exceptional code. many of my comments are just nitpicks 😭 but overall the structure and logic looks amazing, gj 😸

ofc... theres work to be done 😼 lock in and get this cleaned up

@00magikarp 00magikarp added the invalid This doesn't seem right label Aug 16, 2025
@00magikarp
Copy link
Member

^^ do not merge to main ofc

Copy link
Member

@00magikarp 00magikarp left a comment

Choose a reason for hiding this comment

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

better -- a lot better
remove the unneccessary edits, especially to files not related to intake. (if spotlessapply did that its fine)
very good job on getting comments resolved so quickly 😄


class ClosedLoop(val position: Angle) : IntakeRequest

class Uninitialized(): IntakeRequest
Copy link
Member

Choose a reason for hiding this comment

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

remove

Copy link
Author

Choose a reason for hiding this comment

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

wait you need a closed loop, and uninitialized state right?

Copy link
Author

Choose a reason for hiding this comment

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

interesting, shayaan's has "targeting_pos" as a state instead, which makes more sense ig, wb uninitialized?

Copy link
Member

Choose a reason for hiding this comment

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

you need an unitialized state, but not a request. you never request to be uninitialized

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants