Skip to content

Provide preliminary state machine model#39

Merged
taesungh merged 19 commits intomainfrom
Fsm
May 10, 2024
Merged

Provide preliminary state machine model#39
taesungh merged 19 commits intomainfrom
Fsm

Conversation

@ryescholin
Copy link
Copy Markdown
Contributor

Hella merge errors, I can take care of a lil later

@taesungh taesungh changed the title test Provide preliminary state machine model May 5, 2024
@taesungh
Copy link
Copy Markdown
Member

taesungh commented May 5, 2024

Could you rebase with the latest main branch and/or resolve the merge conflicts so we can proceed with review?

@ryescholin
Copy link
Copy Markdown
Contributor Author

U bet, want me to keep/include PT code in fsm?

Comment thread control-station/src/views/Dashboard/Dashboard.tsx Outdated
ryescholin and others added 3 commits May 5, 2024 20:54
Copy link
Copy Markdown
Member

@taesungh taesungh left a comment

Choose a reason for hiding this comment

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

There's a lot of different stuff going on here: could we separate out the portions of the tasks within #18 to reduce the scope of this PR? In particular, it would help to have three separate PRs for #19, #21, and #26 to review smaller chunks. Notably, we'll need a way to periodically emit data from the FSM as part of #19.

Comment thread control-station/src/services/PodSocketClient.ts
Comment thread pod-operation/Cargo.toml Outdated
Comment thread pod-operation/src/components/pressure_transducer.rs
Comment thread pod-operation/src/components/signal_light.rs
Comment thread pod-operation/src/state.rs Outdated
Comment thread pod-operation/src/handlers.rs Outdated
Comment thread pod-operation/src/handlers.rs Outdated
Comment thread pod-operation/src/state.rs Outdated
Comment thread pod-operation/src/main.rs Outdated
Comment thread pod-operation/src/main.rs Outdated
@ryescholin
Copy link
Copy Markdown
Contributor Author

For clarity: I think i can clean up majority of these issues by adding emit function to run periodically, and moving handler functions, enteractions, statetransition functions into fsm (which will get rid of global state value, keep one functional state value within fsm). (ill add in pt, signallight when done) correct me if I'm wrong @taesungh

@taesungh
Copy link
Copy Markdown
Member

taesungh commented May 8, 2024

(ill add in pt, signallight when done)

Let's focus on one step at a time: get the fsm in a clean place first. please do not integrate any actual sensor measurements or peripheral control yet, we can handle that in a separate PR.

@ryescholin
Copy link
Copy Markdown
Contributor Author

thats what i meant lol

@ryescholin
Copy link
Copy Markdown
Contributor Author

Ok, most of stuff is updated, having trouble not using GLOBAL_STATE because of the handlers, lemme know if you have any ideas @taesungh

@taesungh
Copy link
Copy Markdown
Member

taesungh commented May 9, 2024

If the state field for #21 is troubling, could we reduce this PR to just the socket loop handler for #19 and merge that first?

@ryescholin
Copy link
Copy Markdown
Contributor Author

Whatever works best @taesungh

@ryescholin ryescholin linked an issue May 9, 2024 that may be closed by this pull request
Comment thread control-station/src/components/ControlPanel/ControlPanel.tsx Outdated
Comment thread control-station/src/services/PodSocketClient.ts Outdated
Comment thread control-station/src/services/PodSocketClient.ts Outdated
Copy link
Copy Markdown
Member

@taesungh taesungh left a comment

Choose a reason for hiding this comment

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

Update: we can live with the global state situation for now and revisit that next. Few other organizational notes and things that would be nice to separate.

Comment thread control-station/src/components/SensorBoxes/Sensors/SensorBox.tsx
Comment thread control-station/src/services/PodSocketClient.ts Outdated
Comment thread pod-operation/src/main.rs Outdated
Comment thread pod-operation/src/main.rs Outdated
@taesungh
Copy link
Copy Markdown
Member

Could we run cargo fmt once more and call it at that

@ryescholin
Copy link
Copy Markdown
Contributor Author

submitted formatted ver, lemme know if there's anything else I can do to fix anything on this rn @taesungh

Copy link
Copy Markdown
Contributor

@samderanova samderanova left a comment

Choose a reason for hiding this comment

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

Looking good, let's revisit the issues here later.

Copy link
Copy Markdown
Member

@taesungh taesungh left a comment

Choose a reason for hiding this comment

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

Thanks for the continued effort on this. Some areas still we could revisit, particularly the global state variable, transition logic, and client-side handlers.

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.

Connect FSM loop with Socket handler

3 participants