Skip to content

Conversation

@cleorenaud
Copy link
Contributor

@cleorenaud cleorenaud commented Oct 3, 2024

The goal of this PR is to implement the Finite State Machine in the AVState.cpp file to match the description provided in the ERT Wiki

@cleorenaud cleorenaud changed the title Remove blank lines Update FSM Oct 7, 2024
@cleorenaud
Copy link
Contributor Author

I have some questions about the AVState.cpp file:

  • There is a error() function that should be implemented. Should we implement it using cases for each of the state and then check that the conditions to go to the next state are met ?
  • The implementation of the functions fromArmed and fromManual doesn't seem to match the behaviours explained in the FSM on the wiki. (I proposed some alternative implementations but left the one that was already present in the file)
  • There are some conditions that we should check for to trigger a change of state that I don't really know how to implement. For example in the fromDescent function we should check that the vehicle is fully depressurised but which sensors does it correspond to ?
  • Should I add more comments to explain the conditions we are checking for ?

@cleorenaud
Copy link
Contributor Author

It seems that in the ERROR ON GROUND part of the Wiki FSM we are missing a 'from' row as in the diagram we can go to the error on ground state from the armed state.

@marinPh
Copy link
Member

marinPh commented Oct 15, 2024

It seems that in the ERROR ON GROUND part of the Wiki FSM we are missing a 'from' row as in the diagram we can go to the error on ground state from the armed state.

this is correct please add it to this PR

@marinPh
Copy link
Member

marinPh commented Oct 15, 2024

I have some questions about the AVState.cpp file:

  • There is a error() function that should be implemented. Should we implement it using cases for each of the state and then check that the conditions to go to the next state are met ?
  • The implementation of the functions fromArmed and fromManual doesn't seem to match the behaviours explained in the FSM on the wiki. (I proposed some alternative implementations but left the one that was already present in the file)
  • There are some conditions that we should check for to trigger a change of state that I don't really know how to implement. For example in the fromDescent function we should check that the vehicle is fully depressurised but which sensors does it correspond to ?
  • Should I add more comments to explain the conditions we are checking for ?
  1. the error function for now can just return "false" for now we will implement it after
  2. don't take in account the old transitions they were not coded with the old code base in mind
  3. we have sensors that will come from the powerboard, please create #define file that contains sensor information we still don't have for now
  4. I need to take a more thorough look into your documentation for now seems quite good

@cleorenaud
Copy link
Contributor Author

It seems that in the ERROR ON GROUND part of the Wiki FSM we are missing a 'from' row as in the diagram we can go to the error on ground state from the armed state.

this is correct please add it to this PR

I updated the wiki : https://rocket-team.epfl.ch/en/competition/firehorn/avionics/2024_C_AV_FINITE-STATE-MACHINE

uint32_t norm_squared (){
return x*x + y*y + z*z;
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where are those helper functions used ? If we only need the norm in the FSM or high level modules, then it might be better to implement this function in the Vector3 struct in data.h/cpp. Also as it is a common function to any 3D data, implementing it only once would be better. Also why the need of an uint32_t return type ?

adxl2.calibrate();
}

double Vector3::norm() const {
Copy link
Collaborator

@clacassa clacassa Nov 13, 2024

Choose a reason for hiding this comment

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

Why defining this in a different TU than its declaration ? Might be better to define in data.cpp, or even consider making it inline in data.h, as it is a helper function of Vector3.
Also, if used only for comparison, no need to get the sqrt, we can compare it with squared thresholds to avoid this heavy instruction. (Req FSM frequency >= 100Hz).


// Initial state should be INIT
fsm.update(dump);
assert(fsm.getCurrentState() == State::INIT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, check first after constructor call, but might also be good to check after the first fsm.update() the state remains INIT.

{
return State::READY;
}
return State::ARMED;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These conditions overall need some correction. The switch over the telemetry_cmd.id seems deprecated.
The transition ARMED->READY is OK for the moment but will later be determined by the feedback from DPR boards (PR tanks pressure regulators).

Copy link
Member

Choose a reason for hiding this comment

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

To be clear we don't verify the tanks, we just get the go from DPR ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, once we receive PRESSURIZE command from GS and we send PRESSURIZE to the DPRs, we wait for their GO

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed for 'epflrocketteam-1_2024_C_AV_RPI'

Failed conditions
26.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@marinPh marinPh marked this pull request as ready for review November 17, 2024 13:34
@marinPh marinPh merged commit 3bd2571 into main Nov 17, 2024
1 check passed
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.

4 participants