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

[DRAFT] CPU load proof of concept #3

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

DiegoArmstrong
Copy link

No description provided.

@DiegoArmstrong DiegoArmstrong self-assigned this Oct 12, 2024
Comment on lines 10 to 18
bool circ_buf_full(circ_buf_t *p_circ_bbuf_t, uint8_t maxlen) {
return (p_circ_bbuf_t->num_entries == maxlen);
}

bool circ_buf_empty(circ_buf_t *p_circ_bbuf_t) {
return (p_circ_bbuf_t->num_entries == 0);
}

int circ_buf_enqueue(circ_buf_t *pBuffer, float data, uint8_t maxlen) {

Choose a reason for hiding this comment

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

Since these functions are exposed in circular_buffer.h make sure you follow the coding standard for public functions. See the Coding Style here: https://wiki.ubcsolar.com/subteams/embedded/coding_style

Of course this is still a solar sandbox project, however, to make your refactor and integration to production code later, I recommend following the coding style from the beginning.

Copy link
Author

Choose a reason for hiding this comment

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

Unclear on what coding standard isn't being followed?

}

// Insert the data and move the head
pBuffer->pBuffer[pBuffer->head] = data;

Choose a reason for hiding this comment

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

The naming here is a bit confusing since pBuffer is a float* inside the struct and pBuffer is also a circ_buf_t*.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, can definitely see how this could be super confusing to someone who didn't write it.

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