Communities

Writing
Writing
Codidact Meta
Codidact Meta
The Great Outdoors
The Great Outdoors
Photography & Video
Photography & Video
Scientific Speculation
Scientific Speculation
Cooking
Cooking
Electrical Engineering
Electrical Engineering
Judaism
Judaism
Languages & Linguistics
Languages & Linguistics
Software Development
Software Development
Mathematics
Mathematics
Christianity
Christianity
Code Golf
Code Golf
Music
Music
Physics
Physics
Linux Systems
Linux Systems
Power Users
Power Users
Tabletop RPGs
Tabletop RPGs
Community Proposals
Community Proposals
tag:snake search within a tag
answers:0 unanswered questions
user:xxxx search by author id
score:0.5 posts with 0.5+ score
"snake oil" exact phrase
votes:4 posts with 4+ votes
created:<1w created < 1 week ago
post_type:xxxx type of post
Search help
Notifications
Mark all as read See all your notifications »
Q&A

Welcome to Software Development on Codidact!

Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.

Comments on Avoiding checking opcode multiple times in state machine for embedded system

Post

Avoiding checking opcode multiple times in state machine for embedded system

+2
−0

I am implementing a state machine for an electrical relay control board and am trying to follow the principles/structure presented by Lundin here for doing that in C.

My machine has currently four states: init, poll, status, and switch. In the poll state the microcontroller waits to receive an opcode via uart and from that determines which state to go to next. My state diagram without error handling is shown below.

Image_alt_text

Here is a portion of my main function that incorporates the structure of the state machine and state transitions with almost all of the code taken from Lundin's example.

/* main.c */
static cmd_func_t *const state_machine[]  =
{
    [CMD_INIT] = cmd_init,
    [CMD_POLL] = cmd_poll,
    [CMD_SWITCH] = cmd_switch,
    [CMD_STATUS] = cmd_request_status,
};

_Static_assert(sizeof state_machine/sizeof *state_machine == CMD_STATES_N,
		"cmd_state_t and state_machine does not match 1 to 1");

static cmd_state_t evaluate_result(cmd_state_t current_state, cmd_result_t result);
static void error_handler(cmd_result_t error_code);

int main(void)
{
    usart_init(BAUD, USART_PARITY_NONE, USART_STOP_1, 
               USART_BITS_8, USART_SPEED_NORMAL);
    pin_mode(&D24, OUTPUT); /* Enable indication LED */
    static cmd_state_t state = CMD_INIT;
    cmd_result_t result;

    for(;;)
    {
        result = state_machine[state]();	/* Execute function */
        state = evaluate_result(state, result); /* Change state */
    }
}

static cmd_state_t evaluate_result(cmd_state_t current_state, cmd_result_t result)
{
	cmd_state_t next_state = current_state;

	switch(current_state)
	{
		case CMD_INIT:
			if(result == CMD_OK)
			{
				next_state = CMD_POLL;
			}
			break;

		case CMD_POLL:
			if(result == CMD_OK)
			{
				next_state = CMD_SWITCH;
			}

/* More below */

One problem that I face, is that I'm checking the opcode that I receive twice. One time in order to determine which state to transition to from CMD_POLL. And once again in CMD_SWITCH when I need to determine if a relay or an entire channel of relays should be turned on/off. First check is in my cmd_poll() function.

/* cmd.c */
static uint8_t received_opcode = 0x00u;
static uint8_t switch_number = 0u;

cmd_result_t cmd_poll(void)
{
	if(usart_available() > 0u)
	{
		received_opcode = usart_rx();
		switch_number = usart_rx();

		switch(received_opcode)
		{
			case OPCODE_CHANNEL_ON:
				return CMD_OK;
			case OPCODE_CHANNEL_OFF:
				return CMD_OK;
			case OPCODE_RELAY_ON:
				return CMD_OK;
			case OPCODE_RELAY_OFF:
				return CMD_OK;
			case OPCODE_STATUS:
				return CMD_REQUEST;
			default:
				return CMD_UNRECOGNIZED_OPCODE;
		}
		return CMD_UNEXPECTED;
	}
	else
	{
		return CMD_IDLE;
	}
	return CMD_UNEXPECTED; //Should never be reached
}

Next check is in my cmd_switch() function:

/* cmd.c */
cmd_result_t cmd_switch(void)
{
    if(received_opcode == OPCODE_CHANNEL_ON)
    {
        /* Do something */
    }
    else if(received_opcode == OPCODE_CHANNEL_OFF)
    {
        /* Do something else*/
    }
    else if(...)

Is there a smarter way to do this such that I avoid this redundancy of having to check the opcode twice? Preferably a way that is MISRA compliant as well.

History

1 comment thread

I fear you may have misunderstood why state machines are used when receiving from USART. See how you ... (1 comment)
I fear you may have misunderstood why state machines are used when receiving from USART. See how you ...
user253751‭ wrote about 1 month ago

I fear you may have misunderstood why state machines are used when receiving from USART. See how you call usart_rx twice, even if only one byte is available - what does the second call return, if no more bytes are available? And what is the point of cmd_switch being its own state, if it always returns CMD_OK and goes back to poll state? A state is somewhere the machine can wait for a while. Anywhere the machine has to wait should be a state - such as between the opcode and switch number - since the sender might pause before sending the switch number. Anywhere the machine doesn't have to wait shouldn't be a state - such as processing the command (unless the relay takes several polls to switch).