Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/state machine #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions fsm/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Node State Machine
fsm.c implements the state machine for the Node subsystem as shown here:

![State_Diagram_14-5-22 (1)](https://user-images.githubusercontent.com/67641046/169704065-c144127e-2833-4674-a9cf-f29355fdd1c9.png)

In order to compile:
- Ensure that your gcc version is up to date with gcc --version (my version is 11.2.0).
- cd into the /fsm subdirectory and do gcc fsm.c -o fsm
- ./fsm
206 changes: 206 additions & 0 deletions fsm/fsm.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <assert.h>

typedef enum
{
BOOT,
AUTONOMOUS,
MANUAL
} State;

void get_next_state(char input, State **state);
void test_fsm();

int main()
{
char input;
State * state;
State tmp;

State boot = BOOT;

state = &boot;

while (1)
{
printf("\nCurrent state is: %d\n", *state);
printf("Please enter character input: ");
scanf("%c", &input);
if (input == '\n' || input == EOF)
{
continue;
}
if (input == 'q' || input == 'Q')
{
break;
}
get_next_state(input, &state);
tmp = *state;
fflush(stdin);
state = &tmp;
}

test_fsm();
printf("Unit Test Passed");
}

// Unit test asserting correct state transitions
// returns true if pass, false otherwise
void test_fsm()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put tests in another file?

{
char input;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you define somewhere what each char corresponds to, I figured it out after staring at the code for a while but a little key comment would be nice


// Test setup
State * boot;
State boot_val = BOOT;
State * autonomous;
State autonomous_val = AUTONOMOUS;
State * manual;
State manual_val = MANUAL;

// Boot -> Boot transition tests
boot = &boot_val;
input = 'p';
get_next_state(input, &boot);
assert(*boot == boot_val);

boot = &boot_val;
input = 'P';
get_next_state(input, &boot);
assert(*boot == boot_val);

boot = &boot_val;
input = 'w';
get_next_state(input, &boot);
assert(*boot == boot_val);

boot = &boot_val;
input = 'W';
get_next_state(input, &boot);
assert(*boot == boot_val);

// Boot -> Autonomous transition tests
boot = &boot_val;
input = 'a';
get_next_state(input, &boot);
assert(*boot == autonomous_val);

boot = &boot_val;
input = 'A';
get_next_state(input, &boot);
assert(*boot == autonomous_val);

// Boot -> Manual transition tests
boot = &boot_val;
input = 'm';
get_next_state(input, &boot);
assert(*boot == manual_val);

boot = &boot_val;
input = 'M';
get_next_state(input, &boot);
assert(*boot == manual_val);

// Autonomous -> Boot transition tests
autonomous = &autonomous_val;
input = 'p';
get_next_state(input, &autonomous);
assert(*autonomous == boot_val);

autonomous = &autonomous_val;
input = 'P';
get_next_state(input, &autonomous);
assert(*autonomous == boot_val);

autonomous = &autonomous_val;
input = 'w';
get_next_state(input, &autonomous);
assert(*autonomous == boot_val);

autonomous = &autonomous_val;
input = 'W';
get_next_state(input, &autonomous);
assert(*autonomous == boot_val);

// Manual -> Boot transition tests
manual = &manual_val;
input = 'p';
get_next_state(input, &manual);
assert(*manual == boot_val);

manual = &manual_val;
input = 'P';
get_next_state(input, &manual);
assert(*manual == boot_val);

manual = &manual_val;
input = 'w';
get_next_state(input, &manual);
assert(*manual == boot_val);

manual = &manual_val;
input = 'W';
get_next_state(input, &manual);
assert(*manual == boot_val);
}

// Get the next state based on the current state and input
void get_next_state(char input, State **state)
{
State boot = BOOT;
State autonomous = AUTONOMOUS;
State manual = MANUAL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I'm just not used to C code but isn't it redundant to recreate these globals?


int curr_state = (int)**state;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason to reassign this variable that's passed in? Shouldn't you just name the original variable curr_state

Copy link
Collaborator

Choose a reason for hiding this comment

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

also shouldn't you be using camel case not snake case?


if (curr_state == (int)boot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing logic to deny state change if health not good

{
if (input == 'P' || input == 'p' || input == 'W' || input == 'w')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this if is the same for every state, IMO you should just pull it out to the beginning and break early if called

{
*state = &boot;
}

else if (input == 'A' || input == 'a')
{
*state = &autonomous;
}

else if (input == 'M' || input == 'm')
{
*state = &manual;
}
}

else if (curr_state == (int)autonomous)
{
if (input == 'P' || input == 'p' || input == 'W' || input == 'w')
{
*state = &boot;
}

else
{
*state = &autonomous;
Copy link
Collaborator

Choose a reason for hiding this comment

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

am I reading this right as a state change from autonomous to autonomous?

}
}

else if (curr_state == (int)manual)
{
if (input == 'P' || input == 'p' || input == 'W' || input == 'w')
{
*state = &boot;
}

else
{
*state = &manual;
}
}

else
{
printf("Error: Unknown State\n");
}
}