Skip to content

Updated command dispatcher to be a library#44

Open
SiddhartaL wants to merge 1 commit into
mainfrom
cmd-dispatch-update
Open

Updated command dispatcher to be a library#44
SiddhartaL wants to merge 1 commit into
mainfrom
cmd-dispatch-update

Conversation

@SiddhartaL
Copy link
Copy Markdown
Member

All the command dispatcher does is dereference a pointer. The logic of enqueue_command was encapsulated into a single function, defined under src/libraries/cmd_dispatcher/. We now include the header cmd_dispatcher.h to access this function. Further, the implementation adds a logging buffer to use with telemetry.

@zachMahan64
Copy link
Copy Markdown
Member

The switch to the library looks good.

Only issue I can see is that you've replaced the calls to enqueue commands to while loops that repeatedly try to enqueue until success. One failure will generally mean that enqueuing will continue to fail, so any failed enqueuing will generally lead to the tx task being stuck in that (potentially) infinite loop. Even if it doesn't loop forever, we're still going to run the risk of spin locking a task unacceptably long given it could take say 100000 iterations, or more. It's going to be better to try to enqueue once, as it was. Perhaps we could have it so that a tx task instead yields to the FreeRTOS scheduler on a failed enqueue. For now, I think it is better to leave those calls to enqueue logically unchecked besides issuing the warning. So basically this was my long-winded way of telling you to just change all your whiles to ifs for those enqueue checks.

@SiddhartaL
Copy link
Copy Markdown
Member Author

Modifications added to the branch

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.

2 participants