Skip to content

Comments

Coroutines to fsm#3613

Open
StarrryNight wants to merge 23 commits intoUBC-Thunderbots:masterfrom
StarrryNight:coroutines_to_fsm_#2359
Open

Coroutines to fsm#3613
StarrryNight wants to merge 23 commits intoUBC-Thunderbots:masterfrom
StarrryNight:coroutines_to_fsm_#2359

Conversation

@StarrryNight
Copy link
Contributor

@StarrryNight StarrryNight commented Feb 16, 2026

Description

This ticket resolves #3605, which is the first part of #2359 . The following plays are converted from coroutines into FSM:

  • halt test play
  • move test play
  • shoot or chip play

The process includes adding a new *_fsm.h file (also *_fsm.cpp file for shoot or chip play), and refactoring the old play files. Tests are created for halt test play and move test play.

Shoot or chip play is also moved into a subfolder in plays for clarity, and a BUILD file is added as a result.

The completion of #2359 is blocked by 3 other issues recorded in #3605 .

Testing Done

  • All play tests passes
  • all_play builds

Resolved Issues

#3605 , #2359

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@StarrryNight StarrryNight changed the title Coroutines to fsm #2359 Coroutines to fsm Feb 16, 2026
@StarrryNight StarrryNight marked this pull request as ready for review February 16, 2026 02:03
Copy link
Contributor

@adrianchan787 adrianchan787 left a comment

Choose a reason for hiding this comment

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

just finished shoot_or_chip, will review halt_test later, but really nice work i think! basically only thing i can think of is maybe including the comment // src_state + event [guard] / action = dest_state into make_transition_table since the other fsms seem to have that, though that's very small.

Also, why is chip_target an optional? (personal question since i likely am wrong, but just curious because with this code, it doesn't seem to need to be an optional?)

Copy link
Member

@nycrat nycrat left a comment

Choose a reason for hiding this comment

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

Looks good, should probably add doc comments to the fsm classes

Copy link
Member

@nycrat nycrat left a comment

Choose a reason for hiding this comment

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

Still need doc comments for methods on those FSM classes. Look at free_kick_play_fsm.h or ball_placement_play_fsm.h as examples

@StarrryNight
Copy link
Contributor Author

StarrryNight commented Feb 24, 2026

I took a look at different fsm plays and realized many of them (such as offense play) don't have the documentation. I will add the docs for the affected fsms in this PR, but we should open a ticket to write docs for all FSM once all coroutines are converted.

edit: Actually, I will add the other docs in the clean up coroutine to FSM ticket I have when i delete all the coroutines

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.

Convert remaining coroutines plays into FSM

3 participants