Northeastern-Electric-Racing / shepherd_bms

Repo for Shepherd BMS development
5 stars 0 forks source link

141 implement state machine initialization #152

Closed HamzaIqbal69 closed 1 year ago

HamzaIqbal69 commented 1 year ago

141 State Machine needed material in initialization

Ticket # -> Fix X

Added initialization functionality ot each state that needed it in the state machine.

HamzaIqbal69 commented 1 year ago

Bet sounds good

On Tue, Feb 28, 2023 at 3:21 PM dyldonahue @.***> wrote:

@.**** commented on this pull request.

seems mostly fine to me, just wanna shift some declarations to be outside of the functions and floating in statemchine.h

— Reply to this email directly, view it on GitHub https://github.com/Northeastern-Electric-Racing/shepherd_bms/pull/152#pullrequestreview-1318542616, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWEYWBQHS3IDLUMQ3V27HGLWZZM37ANCNFSM6AAAAAAVKCN5EE . You are receiving this because you authored the thread.Message ID: <Northeastern-Electric-Racing/shepherd_bms/pull/152/review/1318542616@ github.com>

dyldonahue commented 1 year ago

nick may have different answer but to me, that just seems like more work. We can transition to/from different variations of states, why run a check every iteration on all of those instead of just requesting a change when needed. it doesnt really change much either way, but a broader transition check would perform a ton more calculations just for the sake of automatically figuring out what state we wanna go into as opposed to requesting into a specific state

nwdepatie commented 1 year ago

Both ways of transitioning are valid, but this is just making it more explicit when we want to change. Instead of having to delve through a shut ton of conditionals, we just change the state whenever the handler detects a certain condition has happened. Also all those conditionals would need to be checked on every loop, when we can just check if it's a valid transition once. Kinda like an event driven system instead of a timer based one sort of in a way

nwdepatie commented 1 year ago

And there will be functionality and checks that are common to all states (I.e. fault checks) but there might be the possibility that transition conditions may change depending on the state. Like if we fault we might want the temperature to go to lower than the maximum allowed before we reboot or something